-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/52877/
-----------------------------------------------------------
(Updated Jan. 9, 2017, 3:29 p.m.)
Review request for mesos, Adam B, Benjamin Bannier, Joris Van Remoortere, 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 (updated)
-----
3rdparty/stout/include/stout/jsonify.hpp
8220d001001e8b9f247c05be58534f1174611aad
Diff: https://reviews.apache.org/r/52877/diff/
Testing
-------
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();
// Randomly generated set of numbers.
std::vector<double> doubles = {
54366462691.1798,
3.35465250645312,
3056184950.05953,
74057564.7741182,
280.445791893924,
3446.63176368415,
419012114.325581,
3464212.51004162,
1.45156507568354,
13304750.4216248,
7716457488648.00,
700533630.650588,
679.659801950981,
307059152.688268,
5615744.28063731,
859.902033900705,
293878.810967451,
284685668.155903,
722683811.462448,
407.682284299325,
9874834.88341080,
4829649.14505646,
3045544.72401146,
1112707.08627010,
8379539.79388719,
3004161.89676627,
3866662.79849617,
3871151.73991937,
4090119.26657417,
4118699.88616345,
2104416.18322520,
9896898.63226234,
5957851.08773457,
3501068.52003430,
7524714.36218293,
4333874.01982647,
9562008.06930384,
3374957.45494027,
5867075.07463260,
815499.697450741,
600936.470830026,
9661425.72632153,
6392256.71537575,
7517969.33139398,
9031912.03425553,
5497593.85752735,
815419.808032205,
5098659.46057626,
930160.667551563,
5970944.98217500,
6166534.92677787,
3541537.67676275,
1291933.13549156,
9185094.72404290,
4507338.03523123,
9559754.89147696,
6152898.39204769,
2358966.41795446,
6520510.92218183,
2201757.78606032,
4960487.80737309,
1784969.91832029,
3858390.23735070,
1439952.27402359,
6407199.91163080,
6130379.95590661,
6427890.23913244,
2128879.29010338,
8175291.42483598,
1587278.27639235,
7493343.68705307,
4853439.25371342,
2564845.15639735,
1415661.63929173,
6388168.79342734,
3003424.90116775,
6915390.10600792,
7115390.65502377,
5288515.90088063,
1209208.86882085,
4483111.91884606,
5974377.97163572,
5821054.89489766,
8284136.84073623,
1607044.34898051,
3255087.31267763,
2305369.43079747,
1282392.57487249,
4884797.49134467,
5119420.62129117,
6276725.07755672,
3054999.92210194,
3594116.58894982,
6691568.49651968,
265562.721872220,
2864878.07276221,
627299.902077148,
5885179.44212665,
7654144.98508934,
590857.599685431
};
state.ResumeTiming();
benchmark::DoNotOptimize(jsonify(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");
*stream_ << trimmed << (trimmed.back() == '.' ? "0" : "");
```
Perfomance
```
Run on (8 X 2800 MHz CPU s)
2017-01-03 15:06:35
Benchmark Time CPU Iterations
-------------------------------------------------
BM_Jsonify 985 ns 986 ns 714227
```
2. `jsonify.hpp` default (it changes the call to `strings::trim` 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';
}
*stream_ << buffer << (buffer[back] == '.' ? "0" : "");
```
Performance
```
Run on (8 X 2800 MHz CPU s)
2017-01-03 15:05:34
Benchmark Time CPU Iterations
-------------------------------------------------
BM_Jsonify 968 ns 969 ns 723417
```
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] {};
int size = snprintf(
buffer,
sizeof(buffer),
"%#.*g",
std::numeric_limits<double>::digits10,
double_);
const char separator =
std::use_facet<std::numpunct<char>>(*stream_.getloc()).decimal_point();
for (int i = 0; i < size; ++i) {
if (buffer[i] == separator) {
buffer[i] = '.';
break;
}
}
int back = size - 1;
for (; back > 0; --back) {
if (buffer[back] != '0') {
break;
}
buffer[back] = '\0';
}
*stream_ << buffer << (buffer[back] == '.' ? "0" : "");
```
Performance
```
Run on (8 X 2800 MHz CPU s)
2017-01-03 15:07:39
Benchmark Time CPU Iterations
-------------------------------------------------
BM_Jsonify 982 ns 981 ns 704544
```
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 2800 MHz CPU s)
2017-01-03 15:09:40
Benchmark Time CPU Iterations
-------------------------------------------------
BM_Jsonify 977 ns 978 ns 715571
```
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);
*stream_ << representation << (representation[back] == '.' ? "0" : "");
```
Performance
```
Run on (8 X 2800 MHz CPU s)
2017-01-03 15:10:52
Benchmark Time CPU Iterations
-------------------------------------------------
BM_Jsonify 982 ns 984 ns 714935
```
6. Proposal 4: Using thread local variables:
```c++
static THREAD_LOCAL std::stringstream* ss = nullptr;
static THREAD_LOCAL bool initialized = false;
if (!initialized) {
ss = new std::stringstream;
ss->imbue(std::locale::classic());
*ss << std::setprecission(std::numeric_limits<double>::digits10)
<< std::showpoint;
initialized = true;
}
*ss << double_;
std::string trimmed = strings::trim(ss->str(), strings::SUFFIX, "0");
*stream_ << trimmed << (trimmed.back() == '.' ? "0" : "");
ss->str("");
```
Performance
```
Run on (8 X 2800 MHz CPU s)
2017-01-03 15:12:37
Benchmark Time CPU Iterations
-------------------------------------------------
BM_Jsonify 980 ns 980 ns 716413
```
Surprisingly, proposal 2 seemed to be the most efficient after multiple runs of
the suite.
Thanks,
Alexander Rojas
