-
Notifications
You must be signed in to change notification settings - Fork 683
Simplify string management. #1189
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
Conversation
Allocate a single memory block for strings, rather than a separate string header and string characters block. In the past strings were split into 8 byte chunks, and large amount of legacy code is designed for that representation. However the current allocator allows block allocation so we don't need those complicated algorithms anymore. This patch is a cleanup rather than an optimization. JerryScript-DCO-1.0-Signed-off-by: Zoltan Herczeg zherczeg.u-szeged@partner.samsung.com
@zherczeg, any effect on memory usage? |
No, because non-ascii strings are not tested by sunspider. This patch also a preparation for allowing 32 bit compressed (actually uncompressed) pointers as well. |
The good thing is the effect of the patch is slightly positive on all string manipulation tests. However since the slow cases are improved mostly, the effect is small (as they should!). |
Nice cleanup! I'm not familiar enough with the string handling code to provide substantial feedback. Great to see that this will bring us a step closer to offering the option of uncompressed pointers as well :) |
Hi @zherczeg , after the patch, some kinds of strings (ECMA_STRING_CONTAINER_HEAP_UTF8_STRING) are not managed by pool chunk (jmem_pools_alloc), and not counted in the related pool_stats. Right? |
Yes. I consider the pool allocator as a legacy system, which should be moved out from the project eventually. |
@zherczeg so in the future, every ecma-type will be directly managed by heap, without the poolman,, right? |
Ideally yes. The current allocator is capable of allocating blocks with any length. The old allocator allocated 64 byte blocks, so it wasted too much space without this pool system. This is not a problem anymore. |
@zherczeg Thanks. I didn't notice that the allocating blocks have no length constraints. Cool! |
LGTM |
1 similar comment
LGTM |
Allocate a single memory block for strings, rather than a separate string header
and string characters block. In the past strings were split into 8 byte chunks,
and large amount of legacy code is designed for that representation. However the
current allocator allows block allocation so we don't need those complicated
algorithms anymore. This patch is a cleanup rather than an optimization.
Small speedup is achieved though. Binary size unchanged.