faster String allocation #29254
faster String allocation #29254
Conversation
only a few %, but essentially removes the cost of computing the sizeclass, so this should save on processor resources (frontend, BTP) also
| @@ -476,7 +477,8 @@ JL_DLLEXPORT jl_value_t *jl_pchar_to_string(const char *str, size_t len) | |||
|
|
|||
| JL_DLLEXPORT jl_value_t *jl_alloc_string(size_t len) | |||
| { | |||
| jl_value_t *s = jl_gc_alloc(jl_get_ptls_states(), sizeof(size_t)+len+1, jl_string_type); | |||
| size_t sz = sizeof(size_t) + len + 1; // add space for trailing \nul protector and size | |||
| jl_value_t *s = jl_gc_alloc_(jl_get_ptls_states(), sz, jl_string_type); // force inlining | |||
There was a problem hiding this comment.
These are starting to get fairly repetitive. Maybe factor out into a common subroutine?
There was a problem hiding this comment.
there's only two of them, so it's not worthwhile
There was a problem hiding this comment.
Also, this method technically is that common subroutine. The code is duplicated in jl_pchar_to_string for performance.
|
@nanosoldier |
|
Something went wrong when running your job: |
|
@nanosoldier |
|
Your benchmark job has completed - possible performance regressions were detected. A full report can be found here. cc @ararslan |
|
Why would this affect allocation numbers? Are those tests known to be flaky, or did I mess up the size-class computation (either old or new)? |
|
@nanosoldier |
|
Your benchmark job has completed - possible performance regressions were detected. A full report can be found here. cc @ararslan |
|
Double-checked the formula, and confirmed the old one was slightly offset, leading to larger allocations than necessary in some rare cases. So we'll add to the benefits of this PR that the size-classes are also now computed correctly 😜. |
Only a few % difference in performance measurements (the majority of time is usually spent in examining the current state of the pool), but this essentially removes the cost of computing the sizeclass, so it should save on processor resources (frontend, BTP) especially.
I'm also hoping this will help unstick #21959, since it makes changing the size classes easy.