-
Notifications
You must be signed in to change notification settings - Fork 125
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
xkbcomp/scanner: avoid extra copies for keynames, keywords, identifiers #638
Conversation
I run the bench but the improvement is a bit disappointing: Runtime: # Master
$ ./build_master/bench-compile-keymap --stdev 0.3 --layout us,de --variant ,neo
mean: 2094 µs; stdev: 0.296366% (target: 0.300000%); last run: compiled 2048 keymaps in 4.292202s; total time: 8.582081s
# This PR
$ ./build_release/bench-compile-keymap --stdev 0.3 --layout us,de --variant ,neo
mean: 2098 µs; stdev: 0.299656% (target: 0.300000%); last run: compiled 1024 keymaps in 2.149264s; total time: 4.299814s Memory: $ memusage ./build_master/compile-keymap --layout us,de --variant ,neo --test 2>&1 | head -n 7
Memory usage summary: heap total: 977972, heap peak: 300995, stack peak: 18720
total calls total memory failed calls
malloc| 23921 622500 0
realloc| 2287 295776 0 (nomove:81, dec:0, free:0)
calloc| 577 59696 0
free| 28676 977972
# This PR
$ memusage ./build_release/compile-keymap --layout us,de --variant ,neo --test 2>&1 | head -n 7
Memory usage summary: heap total: 977968, heap peak: 300991, stack peak: 18720
total calls total memory failed calls
malloc| 23921 622496 0
realloc| 2287 295776 0 (nomove:81, dec:0, free:0)
calloc| 577 59696 0
free| 28676 977968 Did I miss something? |
No, this PR is not much of an optimization. I will post a PR shortly that builds on this one and shows an improvement. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That makes more sense, thanks for the explanation.
Just 2 nitpicks left for me.
Needed for next commit, but good regardless. No noticeable effect on performance. Signed-off-by: Ran Benita <[email protected]>
The tokens don't have escapes so no need to use the `buf` for them. Signed-off-by: Ran Benita <[email protected]>
e953159
to
b4ae85e
Compare
The tokens don't have escapes so no need to use the
buf
for them.