Comments (17)
Quickly benchmarking calls to base62() revealed a 10~15% performance difference on my computer.
Locally allocated results
$ g++ sole.cxx -std=c++11 && ./a.out
Benchmarking... 263582 base62/sec
Benchmarking... 268503 base62/sec
Benchmarking... 267937 base62/sec
Benchmarking... 258611 base62/sec
Benchmarking... 266666 base62/sec
Benchmarking... 267978 base62/sec
Benchmarking... 266684 base62/sec
Benchmarking... 248146 base62/sec
Benchmarking... 250766 base62/sec
Cached std::string results
$ g++ sole.cxx -std=c++11 && ./a.out
Benchmarking... 286939 base62/sec
Benchmarking... 284413 base62/sec
Benchmarking... 283041 base62/sec
Benchmarking... 290042 base62/sec
Benchmarking... 289638 base62/sec
Benchmarking... 285924 base62/sec
Benchmarking... 286872 base62/sec
Benchmarking... 292314 base62/sec
Benchmarking... 284472 base62/sec
from sole.
Just found something else inside rebase,
From: res = std::string() + basemap[int(rem)] + res;
To: res = basemap[int(rem)] + res;
Results:
$ g++ sole.cxx -std=c++11 && ./a.out
Benchmarking... 325883 base62/sec
Benchmarking... 328972 base62/sec
Benchmarking... 326594 base62/sec
Benchmarking... 333665 base62/sec
Benchmarking... 334059 base62/sec
Benchmarking... 312550 base62/sec
Benchmarking... 328770 base62/sec
Benchmarking... 334422 base62/sec
Benchmarking... 327754 base62/sec
Another ~15% on top of the previous benchmark (with the cached std::string).
from sole.
Benchmarking... 1902357 base62/sec
Benchmarking... 1986467 base62/sec
Benchmarking... 2000317 base62/sec
Benchmarking... 2019920 base62/sec
Benchmarking... 1984543 base62/sec
Benchmarking... 1915283 base62/sec
Woah, almost more than 6x performance improvement for the base62() apparently, that's what I call an optimization 👍
from sole.
@r-lyeh Was that really a fix though?
As you mentioned yourself this was probably better ignored as it deals with trivial objects (safe to destroy on-exit, regardless of order).
This is a "run-time" vs "on-exit" performance decision, and in my experience most people would gladly accept the trade-off.
After the "fix", at a first glance the performance seems atrocious, a single call of uuid::base62():
- Creates a new (const char*) string
- Creates a new std::string for each rebase(...)
- Each std::string makes a copy of the newly created (const char*) string
So for example, the base62() method will allocate the string 3 times where just a reference to the static std::string would have been enough.
In applications where objects and their uuid's need to be serialized, this looks unacceptable.
from sole.
Super. I 'll have a commit to this fix asap, unless somebody else is faster at PRs :)
Thanks @Edensan to take the time to measure it
from sole.
hey @zammbi, I just checked and the base62optim branch wont pass tests. Gotta review the rebuild(b62) method and let you know then
edit: it's base62()
actually
from sole.
I'm not sure that this warning is really a problem. It's probably better to ignore it.
The fix done, now makes it non static, so it creates a char * every time.
My suggestion is to add static, constexpr and ignore "exit-time declaration" warnings for this method.
from sole.
Have anyone measured/benchmarked both implementations before blaming at the fix? :)
I guess the performance is similar in both cases: it was 1 global string access + 3 string allocs before, and it is a local access (to a likely inlined variable) + 3 string allocs now. But then again, it would need some benchmarking before taking any further decision.
It could be better if I would have done a std::string(base62, sizeof(base62)/sizeof(base62[0]))
instead, though. That's true.
from sole.
Months ago I did do some benchmarks to improve performance in our application. UUID creation was the slowest part, the optimisations I suggested above seemed to help improve it, at least in my benchmarks.
from sole.
I guess the performance is similar in both cases: it was 1 global string access + 3 string allocs before, and it is a local access (to a likely inlined variable) + 3 string allocs now
Uuuh, what? I have a feeling we are not talking about the same thing.
rebase( ..., const std::string &basemap )
This requires a std::string as a parameter, which means a cast/conversion to std::string is required for this to even compile. AFAIK new std::string instances will re-allocate the original string and keep their own internal copy.
If before we were accessing a global std::string instance and feeding it to the rebase, no casts were being required and thus removing the need for unnecessary string allocations.
from sole.
Great find @Edensan. I might have to do that quick edit for our app.
from sole.
@Edensan can you benchmark that branch please? I'm curious to see the results!
from sole.
My guess is that re-allocating strings in that loop was just killing the performance, I see you're using a char buffer now and doing a single std::string allocation, good.
from sole.
What an increase! Nice work @r-lyeh
And that's without const char base62[]
being static.
from sole.
I wonder if base62 remains 100% intact and thus is backward compatible. I am 98% sure it is safe to use. It would be great if any of you guys mind to apply the branch and test your unit-test suites with it :)
from sole.
@r-lyeh Got a warning that this line is not being used.
const char base62[] =
"0123456789"
"ABCDEFGHIJKLMNOPQRSTUVWXYZ"
"abcdefghijklmnopqrstuvwxyz";
But all my tests run fine. Performance is looking good. Time to merge the changes into master after fixing this warning?
from sole.
Merged
from sole.
Related Issues (20)
- Support forward declaration HOT 3
- Build fails when building on GCC mac. HOT 2
- Better performance for uuid4 HOT 4
- msys2 compile warinig HOT 1
- Add method to check if string is a valid guid HOT 2
- uuid4 does not seem to be thread safe.
- uuid4 seems unsafe base on C++ standards HOT 1
- Compile error in C++17 HOT 6
- Warning: possible misuse of comma operator here HOT 1
- need support for android platform HOT 1
- Tag a release HOT 1
- Tag release v1.0.2 HOT 2
- error: call to 'clock_gettime' is ambiguous HOT 4
- socket leak HOT 1
- Call to member function "transform" is ambiguous HOT 5
- time to create a new release?
- error: type of thread-local variable has non-trivial destruction HOT 5
- -Wunused-value compiler warning HOT 2
- Github url in the comments HOT 1
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from sole.