Conversation
3f98ef2 to
fce52ee
Compare
snowleopard
left a comment
There was a problem hiding this comment.
Looks good! I am going to benchmark this change internally too.
|
What about It's fine to leave them for another PR, just wondering if you've got plans. |
I have an upcoming PR indeed. |
fce52ee to
4048e01
Compare
93a1448 to
73ea380
Compare
3a26c8b to
1580f4d
Compare
|
The current version of this PR still shows as a slight regression in my benchmarks. @rgrinberg What are the results on your end? |
|
Runtime performance shows a slight improvement but it's well within the noise range. The GC stats are quite accurate and they do show a small improvement across the board, but that isn't necessarily better for performance. |
Hmm, I wonder if we're seeing different results because we're using different sets of rules. I'm also somewhat surprised to see a small regression in my benchmarks. It seems like this PR is mostly changing things for the better, so I'm kind of puzzled. |
|
I'm not surprised that this change isn't a performance improvement, but I'm definitely surprised there's any regression. I imagined there would be no difference at all either way. I guess we should remember that OCaml hash tables aren't particularly space efficient anyway and removing a single word isn't going to change all that much. We should note that our benchmarks are a bit biased against watch mode though. In watch mode, consuming less memory is more important because it makes subsequent operations faster. While in our measurements, we only care about the first completion build and do not care about any subsequent memory usage. |
|
Just a quick update: I've started some more benchmarks. |
|
Hah, now I'm seeing pretty consistent small improvements across most benchmarks! Maybe I botched it the first time. Roughly: -0.1-0.2% in terms of allocations and -0.1-0.5% in terms of time. |
|
@rgrinberg Btw, don't forget to switch to |
Just to add: with this change included, benchmarks now 2% speed-up for from-cache and null builds! |
98c16f8 to
76d37ba
Compare
Use a more memory efficient path table. Instead of using the variant for the key, combine 3 tables all for the individual paths. This makes the empty table a little more bloated (3x bigger), but gives us a saving of 2 words for every single key we store. Signed-off-by: Rudi Grinberg <me@rgrinberg.com> <!-- ps-id: e86dc1fe-0661-4897-9444-2dfd5f99f050 -->
76d37ba to
07a6f95
Compare
#8052 updated the representation for these, but didn't bump the version. Signed-off-by: Rudi Grinberg <me@rgrinberg.com> <!-- ps-id: 72ce4eb6-2623-4c8e-9933-92a9209f25d1 -->
#8052 updated the representation for these, but didn't bump the version. Signed-off-by: Rudi Grinberg <me@rgrinberg.com>
Use a more memory efficient path table. Instead of using the variant for
the key, combine 3 tables all for the individual paths.
This makes the empty table a little more bloated (3x bigger), but gives
us a saving of 2 words for every single key we store.
Signed-off-by: Rudi Grinberg me@rgrinberg.com