-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/52877/
-----------------------------------------------------------
Review request for mesos, Adam B, Benjamin Bannier, and Michael Park.
Bugs: MESOS-6349
https://issues.apache.org/jira/browse/MESOS-6349
Repository: mesos
Description
-------
Resolves an isse with the JSON serialization functions where floats
would use the process active locale as a decimal separator instead
of the JSON conformant period (`.`).
Diffs
-----
3rdparty/stout/include/stout/json.hpp
9734561549eea867d57f14a0ab71febeb9dddc06
3rdparty/stout/include/stout/jsonify.hpp
8220d001001e8b9f247c05be58534f1174611aad
Diff: https://reviews.apache.org/r/52877/diff/
Testing
-------
Three types of tests were used for this issue. The first just run our current
test suite (`make check`). The second verifies that the proposed solutions
are able to generate a locale independent output and the third consist of a
benchmark of all the solutions.
The verification that the proposed solutions produce locale independent JSON,
the following program was used (with manual verification of the generated
output):
```c++
#include <stout/json.hpp>
#include <clocale>
#include <iostream>
#include <locale>
int main(int argc, char **argv) {
// Set locale to German.
std::setlocale(LC_ALL, "de_DE.UTF-8");
std::cout.imbue(std::locale("de_DE.UTF-8"));
JSON::Object object;
object.values["float"] = 1234567890.12345;
std::cout << stringify(object) << '\n';
return 0;
}
```
In order to test the different options the following benchmarking program was
written using [Google benchmark](https://github.com/google/benchmark):
```c++
#include <benchmark/benchmark_api.h>
#include <stout/json.hpp>
#include <iostream>
#include <random>
#include <string>
#include <sstream>
static void BM_Jsonify(benchmark::State& state) {
while (state.KeepRunning()) {
state.PauseTiming();
JSON::Object object;
object.values["float00"] = 1234567890.12345;
object.values["float01"] = 123456789.012345;
object.values["float02"] = 12345678.9012345;
object.values["float03"] = 1234567.89012345;
object.values["float04"] = 123456.789012345;
object.values["float05"] = 12345.6789012345;
object.values["float06"] = 1234.56789012345;
object.values["float07"] = 123.456789012345;
object.values["float08"] = 12.3456789012345;
object.values["float09"] = 1.23456789012345;
object.values["float10"] = 1234567890.12345;
object.values["float11"] = 123456789.012345;
object.values["float12"] = 12345678.9012345;
object.values["float13"] = 1234567.89012345;
object.values["float14"] = 123456.789012345;
object.values["float15"] = 12345.6789012345;
object.values["float16"] = 1234.56789012345;
object.values["float17"] = 123.456789012345;
object.values["float18"] = 12.3456789012345;
object.values["float19"] = 1.23456789012345;
object.values["float20"] = 1234567890.12345;
object.values["float21"] = 123456789.012345;
object.values["float22"] = 12345678.9012345;
object.values["float23"] = 1234567.89012345;
object.values["float24"] = 123456.789012345;
object.values["float25"] = 12345.6789012345;
object.values["float26"] = 1234.56789012345;
object.values["float27"] = 123.456789012345;
object.values["float28"] = 12.3456789012345;
object.values["float29"] = 1.23456789012345;
object.values["float30"] = 1234567890.12345;
object.values["float31"] = 123456789.012345;
object.values["float32"] = 12345678.9012345;
object.values["float33"] = 1234567.89012345;
object.values["float34"] = 123456.789012345;
object.values["float35"] = 12345.6789012345;
object.values["float36"] = 1234.56789012345;
object.values["float37"] = 123.456789012345;
object.values["float38"] = 12.3456789012345;
object.values["float39"] = 1.23456789012345;
object.values["float40"] = 1234567890.12345;
object.values["float41"] = 123456789.012345;
object.values["float42"] = 12345678.9012345;
object.values["float43"] = 1234567.89012345;
object.values["float44"] = 123456.789012345;
object.values["float45"] = 12345.6789012345;
object.values["float46"] = 1234.56789012345;
object.values["float47"] = 123.456789012345;
object.values["float48"] = 12.3456789012345;
object.values["float49"] = 1.23456789012345;
state.ResumeTiming();
benchmark::DoNotOptimize(stringify(object));
}
}
BENCHMARK(BM_Jsonify);
BENCHMARK_MAIN();
```
The program creates a JSON object with 50 floats in it, and then it creates
its string representation. Since the algorithms in `json.hpp` and `jsonify.hpp`
are similar but not the same, they were benchmarked as different. While the
original solutions are non solutions since they produce erroneous results if
localization is active, they constitute a baseline for comparison with the
alternatives. These are the benchmarks for the original algorithms and their
alternatives:
1. `json.hpp` default:
```c++
char buffer[50] {};
snprintf(
buffer,
sizeof(buffer),
"%#.*g",
std::numeric_limits<double>::digits10,
number.value);
std::string trimmed = strings::trim(buffer, strings::SUFFIX, "0");
return out << trimmed << (trimmed.back() == '.' ? "0" : "");
```
Perfomance
```
Run on (8 X 1000 MHz CPU s)
2016-10-14 13:25:01
Benchmark Time(ns) CPU(ns) Iterations
-------------------------------------------
BM_Jsonify 67878 67597 10516
```
2. `jsonify.hpp` default (it changes the call to `strings::strim` in order to
avoid string copies:
```c++
char buffer[50] {};
const int size = snprintf(
buffer,
sizeof(buffer),
"%#.*g",
std::numeric_limits<double>::digits10,
number.value);
int back = size - 1;
for (; back > 0; --back) {
if (buffer[back] != '0') {
break;
}
buffer[back] = '\0';
}
return out << buffer << (buffer[back] == '.' ? "0" : "");
```
Performance
```
Run on (8 X 1000 MHz CPU s)
2016-10-14 13:27:32
Benchmark Time(ns) CPU(ns) Iterations
-------------------------------------------
BM_Jsonify 65511 65204 10793
```
3. Proposal 1, manually searching for a character which is not expected (and it
is assumed to be the decimal separator) and replacing it:
```c++
char buffer[50] {};
snprintf(
buffer,
sizeof(buffer),
"%#.*g",
std::numeric_limits<double>::digits10,
number.value);
for (char &ch : buffer) {
if (!isdigit(ch) && ch != '-' && ch != '.' && ch!= '\0') {
ch = '.';
break;
}
}
std::string trimmed = strings::trim(buffer, strings::SUFFIX, "0");
return out << trimmed << (trimmed.back() == '.' ? "0" : "");
```
Performance
```
Run on (8 X 1000 MHz CPU s)
2016-10-14 13:32:04
Benchmark Time(ns) CPU(ns) Iterations
-------------------------------------------
BM_Jsonify 69483 69201 9754
```
4. Proposal 2, the C++ approach, use streams and stream manipulators:
```c++
std::stringstream ss;
ss.imbue(std::locale::classic());
ss << std::setprecision(std::numeric_limits<double>::digits10)
<< std::showpoint << number.value;
std::string trimmed = strings::trim(ss.str(), strings::SUFFIX, "0");
return out << trimmed << (trimmed.back() == '.' ? "0" : "");
```
Performance
```
Run on (8 X 1000 MHz CPU s)
2016-10-14 13:33:22
Benchmark Time(ns) CPU(ns) Iterations
-------------------------------------------
BM_Jsonify 82363 82337 8591
```
5. Proposal 3, try to avoid copies (there are still some being created):
```c++
std::stringstream ss;
ss.imbue(std::locale::classic());
ss << std::setprecision(std::numeric_limits<double>::digits10)
<< std::showpoint << number.value;
std::string representation = ss.str();
int back = representation.size() - 1;
for (; back > 0; --back) {
if (representation[back] != '0') {
break;
}
}
representation.resize(back + 1);
out << representation << (representation[back] == '.' ? "0" : "");
```
Performance
```
Run on (8 X 1000 MHz CPU s)
2016-10-14 13:34:40
Benchmark Time(ns) CPU(ns) Iterations
-------------------------------------------
BM_Jsonify 94267 94235 7166
```
Thanks,
Alexander Rojas
