dump() performance degradation in v2

[duncanwerner]

(This should maybe go into #202?)

Testing a switch from v1.1 -> v2.0 I found fairly significant performance degradation in dump() with a large set of doubles. Here's a sample implementation:

json data;
std::vector< double > vec(5e5);
srand(1);
for( int i = 0; i< 5e5; i++ ){ vec[i] = rand();}
data = vec;
std::string str = data.dump();

compiled with vs2015 on windows. Using v1.1 the dump call takes ~550ms. With 2.0 it takes ~25000ms (50x). I haven't tested on any other platforms (although I can do that if it's helpful).

Granted this is an edge case, but it's a pretty dramatic change.

[nlohmann]

The only difference in the dump() function seems to be the handling of locales:

1.1.0:

case value_t::number_float:
{
    // If the number is an integer then output as a fixed with with
    // precision 1 to output "0.0", "1.0" etc as expected for some
    // round trip tests otherwise  15 digits of precision allows
    // round-trip IEEE 754 string->double->string; to be safe, we
    // read this value from
    // std::numeric_limits<number_float_t>::digits10
    if (std::fmod(m_value.number_float, 1) == 0)
    {
        o << std::fixed << std::setprecision(1);
    }
    else
    {
        // std::defaultfloat not supported in gcc version < 5
        o.unsetf(std::ios_base::floatfield);
        o << std::setprecision(std::numeric_limits<double>::digits10);
    }
    o << m_value.number_float;
    return;
}

2.0.0:

case value_t::number_float:
{
    if (m_value.number_float == 0)
    {
        // special case for zero to get "0.0"/"-0.0"
        o << (std::signbit(m_value.number_float) ? "-0.0" : "0.0");
    }
    else
    {
        // Otherwise 6, 15 or 16 digits of precision allows
        // round-trip IEEE 754 string->float->string,
        // string->double->string or string->long
        // double->string; to be safe, we read this value from
        // std::numeric_limits<number_float_t>::digits10
        std::stringstream ss;
        ss.imbue(std::locale(std::locale(), new DecimalSeparator));  // fix locale problems
        ss << std::setprecision(std::numeric_limits<double>::digits10)
           << m_value.number_float;
        o << ss.str();
    }
    return;
}

[gregmarr]

Since there's a memory allocation and deallocation around every number_float conversion, that could explain it.
Should the imbue instead just be done once on the stringstream at the top level dump call, and then just do the setprecision and o << m_value.number_float; as before?

[nlohmann]

Definitely!

[nlohmann]

@duncanwerner Could you please check again with branch https://github.com/nlohmann/json/tree/feature/issue272? There, I moved the locale handling outside the core of the dump function so that the locale is only adjusted once per call.

[gregmarr]

@nlohmann Looks like operator<< should save the locale returned by imbue and restore it after the dump call.

[duncanwerner]

Looks great. In perf testing, roughly

      1.1    2.0      branch
mean  564.4  24298.0  584.5

also works in my "real" application, but harder to benchmark. Thanks!

[nlohmann]

@gregmarr Totally forgot about that. I fixed it and added a unit test.

@duncanwerner Thanks for checking!

[nlohmann]

Thanks a lot. I shall make a release soon.

[gregmarr]

We're not quite back at the performance of V1.0.0 yet.
One more minor change:

        std::stringstream ss;
        ss.imbue(std::locale(std::locale(), new DecimalSeparator));

This creation of the locale every time is still pretty expensive.

        std::stringstream ss;
        const static std::locale loc(std::locale(), new DecimalSeparator);
        ss.imbue(loc);

With this change, our performance is back to the v1.0.0 level on our json heavy tests.

[gregmarr]

@nlohmann PTAL

[gregmarr]

Thanks a lot, we'll give this a shot.