kazuho / picojson

a header-file-only, JSON parser serializer in C++
BSD 2-Clause "Simplified" License
1.12k stars 221 forks source link

Replace handwriten iterator loop with std::for_each. #85

Closed BillyONeal closed 8 years ago

BillyONeal commented 8 years ago

Results in 2x debug mode performance improvement for MSVC++, /Od /RTC1.

kazuho commented 8 years ago

Thank you for the PR.

Can the issue be solved in other ways? For example, can you make the original code run faster by caching the results of s.end() and *i into local variables?

I am not generally against the idea of making a debug build run faster, but if that is the case, would like to see a PR that guarantees (or is reasonable to expect) that the performance of the debug build on other platforms do not get sacrificed.

kazuho commented 8 years ago

relates to #84

AndiDog commented 8 years ago

What's the difference for release mode? Debug doesn't count at all.

BillyONeal commented 8 years ago

@kazuho

Yes, the majority of the win here is by not calling end() n times in the loop. std::for_each can still do better than the handwritten loop because it can do the range check once, and then unwrap the iterators to raw pointers before iterating.

@AndiDog

I didn't observe any changes in release mode perf with this change.

kazuho commented 8 years ago

@BillyONeal

std::for_each can still do better than the handwritten loop because it can do the range check once, and then unwrap the iterators to raw pointers before iterating.

That's a good point. Merged to master.

int19h commented 8 years ago

FWIW, the difference between debug and release builds in this case was two orders of magnitude for some workloads - which was making debug builds outright unusable.