-
Notifications
You must be signed in to change notification settings - Fork 18
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
small optimisation of the lazy memory model #433
base: main
Are you sure you want to change the base?
Conversation
The compiler can't optimise anything here. Is here any reason to use a |
This looks great! I think it will help alot!
It's currently only available here: wasm-se-bench. You can change |
@lthls, thanks for your answer!
It is not meant to be shared for now, but this is probably going to be required for #434. I'll switch to a mutable field for the time being. Thanks for your detailed answer! I have one more question, do you think it could be possible to implement an efficient "nullable ref" type as @¢hambart did in nullable array? Or is there something fundamentally different between references and arrays that would prevent this?
What do you think about doing something like test-comp ? I.e. having the all Wasm code in a git submodule and just keep the OCaml code needed to run the benchmarks. I believe most of the OCaml code can be shared and it's nice to get an unified way to run benchmarks. I'll have a look at your |
You could literally implement it as a nullable array of size 1. You can also write a specialised version with a simple mutable record, but if you want the same guarantees as @chambart's arrays (compatibility with Marshalling) you will likely end up with something that is very similar to a nullable array of size 1. |
Thanks. We don't care about Marshalling compatibility, I'll try to implement it as a record then. |
I initially put the tools' comparative evaluation in the wasm-se-bench repo so that you wouldn’t have to deal with my spaghetti code 😅 However, I’ve properly separated the
We can probably just reuse the current runner with a few adjustments since I based my runner on yours 😅 |
This should be an optimization.
I'm not sure if the compiler will properly optimize
{ data : (Int32.t, Value.int32) Hashtbl.t option ref
. It could remove one layer of indirection by "merging" theoption ref
part but I'm not sure about it (we should ask @chambart and @lthls).The idea is the following:
When traversing the list of parents, this allows to switch from:
Empty -> NonEmptyA -> NonEmptyB -> Empty -> Empty -> NonEmptyC -> Empty -> NonEmptyD
To:
Empty -> NonEmptyA -> NonEmptyB -> NonEmptyC -> NonEmptyD
The invariant is that only the youngest and the oldest element of the list can be empty. Said otherwise, when looking at the whole tree, only the root node and the leaves may be empty. This should reduce allocation and allow to have less element to traverse and try in the list.
cc @filipeom
(I would like to benchmark this on Test-Comp but also running it in B-Tree would be probably more interesting, but Idk how to run this one..)