Skip to content

Commit

Permalink
Refactor.
Browse files Browse the repository at this point in the history
  • Loading branch information
trivialfis committed Jul 25, 2019
1 parent e7cf2b2 commit 2b087ce
Show file tree
Hide file tree
Showing 3 changed files with 66 additions and 65 deletions.
4 changes: 2 additions & 2 deletions include/xgboost/json.h
Original file line number Diff line number Diff line change
Expand Up @@ -271,8 +271,8 @@ struct StringView {
size_t size() const { return size_; } // NOLINT
// Copies a portion of string. Since we don't have std::from_chars and friends here, so
// copying substring is necessary for appending `\0`. It's not too bad since string by
// default has small vector optimization, for numeric value where max length is 17, the
// optimization should be enabled by most if not all modern compiler.
// default has small vector optimization, which is enabled by most if not all modern
// compilers for numeric values.
std::string substr(size_t beg, size_t n) const { // NOLINT
CHECK_LE(beg, size_);
return std::string {str_ + beg, n < (size_ - beg) ? n : (size_ - beg)};
Expand Down
60 changes: 53 additions & 7 deletions include/xgboost/json_io.h
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,18 @@
#include <locale>

namespace xgboost {

template <typename Allocator>
class FixedPrecisionStreamContainer : public std::basic_stringstream<
char, std::char_traits<char>, Allocator> {
public:
FixedPrecisionStreamContainer() {
this->precision(std::numeric_limits<Number::Float>::max_digits10);
}
};

using FixedPrecisionStream = FixedPrecisionStreamContainer<std::allocator<char>>;

class JsonReader {
protected:
size_t constexpr static kMaxNumLength =
Expand Down Expand Up @@ -117,16 +129,50 @@ class JsonReader {
}
};

template <typename Allocator>
class FixedPrecisionStreamContainer : public std::basic_stringstream<
char, std::char_traits<char>, Allocator> {
class JsonWriter {
static constexpr size_t kIndentSize = 2;
FixedPrecisionStream convertor_;

size_t n_spaces_;
std::ostream* stream_;
bool pretty_;

public:
FixedPrecisionStreamContainer() {
this->precision(std::numeric_limits<Number::Float>::max_digits10);
JsonWriter(std::ostream* stream, bool pretty) :
n_spaces_{0}, stream_{stream}, pretty_{pretty} {}

virtual ~JsonWriter() = default;

void NewLine() {
if (pretty_) {
*stream_ << u8"\n" << std::string(n_spaces_, ' ');
}
}
};

using FixedPrecisionStream = FixedPrecisionStreamContainer<std::allocator<char>>;
void BeginIndent() {
n_spaces_ += kIndentSize;
}
void EndIndent() {
n_spaces_ -= kIndentSize;
}

void Write(std::string str) {
*stream_ << str;
}
void Write(StringView str) {
stream_->write(str.c_str(), str.size());
}

void Save(Json json);

virtual void Visit(JsonArray const* arr);
virtual void Visit(JsonObject const* obj);
virtual void Visit(JsonNumber const* num);
virtual void Visit(JsonRaw const* raw);
virtual void Visit(JsonNull const* null);
virtual void Visit(JsonString const* str);
virtual void Visit(JsonBoolean const* boolean);
};
} // namespace xgboost

#endif // XGBOOST_JSON_IO_H_
67 changes: 11 additions & 56 deletions src/common/json.cc
Original file line number Diff line number Diff line change
Expand Up @@ -10,51 +10,6 @@

namespace xgboost {

class JsonWriter {
static constexpr size_t kIndentSize = 2;
FixedPrecisionStream convertor_;

size_t n_spaces_;
std::ostream* stream_;
bool pretty_;

public:
JsonWriter(std::ostream* stream, bool pretty) :
n_spaces_{0}, stream_{stream}, pretty_{pretty} {}

virtual ~JsonWriter() = default;

void NewLine() {
if (pretty_) {
*stream_ << u8"\n" << std::string(n_spaces_, ' ');
}
}

void BeginIndent() {
n_spaces_ += kIndentSize;
}
void EndIndent() {
n_spaces_ -= kIndentSize;
}

void Write(std::string str) {
*stream_ << str;
}
void Write(StringView str) {
stream_->write(str.c_str(), str.size());
}

void Save(Json json);

virtual void Visit(JsonArray const* arr);
virtual void Visit(JsonObject const* obj);
virtual void Visit(JsonNumber const* num);
virtual void Visit(JsonRaw const* raw);
virtual void Visit(JsonNull const* null);
virtual void Visit(JsonString const* str);
virtual void Visit(JsonBoolean const* boolean);
};

void JsonWriter::Save(Json json) {
json.ptr_->Save(this);
}
Expand Down Expand Up @@ -558,12 +513,9 @@ Json JsonReader::ParseObject() {
Json JsonReader::ParseNumber() {
std::string substr = raw_str_.substr(cursor_.Pos(), kMaxNumLength);
size_t pos = 0;

Number::Float number{0};
if (std::is_same<Number::Float, float>::value) {
number = std::stof(substr, &pos);
} else {
number = std::stod(substr, &pos);
}
number = std::stof(substr, &pos);
for (size_t i = 0; i < pos; ++i) {
GetNextChar();
}
Expand Down Expand Up @@ -597,25 +549,28 @@ Json JsonReader::ParseBoolean() {
return Json{JsonBoolean{result}};
}

class GlobalUtf8Locale {
// This is an ad-hoc solution for writing numeric value in standard way. We need to add
// something locale independent way of writing stream.
// FIXME(trivialfis): Remove this.
class GlobalCLocale {
std::locale ori_;

public:
GlobalUtf8Locale() : ori_{std::locale()} {
GlobalCLocale() : ori_{std::locale()} {
std::string const name {"C"};
try {
std::locale::global(std::locale(name.c_str()));
} catch (std::runtime_error const& e) {
LOG(FATAL) << "Failed to set locale: " << name;
}
}
~GlobalUtf8Locale() {
~GlobalCLocale() {
std::locale::global(ori_);
}
};

Json Json::Load(StringView str, bool ignore_specialization) {
GlobalUtf8Locale guard;
GlobalCLocale guard;
LOG(WARNING) << "Json serialization is still experimental."
" Output schema is subject to change in the future.";
JsonReader reader(str, ignore_specialization);
Expand All @@ -628,7 +583,7 @@ Json Json::Load(StringView str, bool ignore_specialization) {
}

Json Json::Load(JsonReader* reader) {
GlobalUtf8Locale guard;
GlobalCLocale guard;
common::Timer t;
t.Start();
Json json{reader->Load()};
Expand All @@ -638,7 +593,7 @@ Json Json::Load(JsonReader* reader) {
}

void Json::Dump(Json json, std::ostream *stream, bool pretty) {
GlobalUtf8Locale guard;
GlobalCLocale guard;
LOG(WARNING) << "Json serialization is still experimental."
" Output schema is subject to change in the future.";
JsonWriter writer(stream, true);
Expand Down

0 comments on commit 2b087ce

Please sign in to comment.