Three hot-path fixes for atom-heavier code - #2388
Open
alii wants to merge 3 commits into
Open
Conversation
The exact-equality opcodes (is_eq_exact, is_not_eq_exact and the select_val jump table that every `case` on atoms compiles to) always went through term_compare. For two different atoms term_compare fetches both atom names from the atom table and compares them byte by byte, so every non-matching clause of an atom `case` cost two atom-table lookups and a memcmp (plus the table lock on SMP builds). Two terms with identical bits are `=:=` equal, and two *immediate* terms (atoms, small integers, nil, local pids, ...) with different bits are never `=:=` equal, since immediates are canonical. term_exact_eq_fast() decides those two cases inline; only when a side is boxed or a list do we fall back to term_compare, so behaviour is unchanged. Profiling a Gleam-compiled interpreter (Arc, a JavaScript engine) under AtomVM showed 78% of samples in atom_table_cmp_using_atom_index called from OP_SELECT_VAL. A standalone loop doing a `case` over eight atoms goes from 67ms to 24ms; the interpreter's hot loops 2-3x. Signed-off-by: Alistair Smith <hi@alistair.sh>
Heap fragments are created whenever a term is built without a GC-safe
point: a compound literal decoded from the module literal table (every
single use of one), a NIF result, a received message. Any fragment at all
then forced a shrinking collection at the next `deallocate`, NIF call or
allocation ("if (ctx->heap.root->next) ... MEMORY_FORCE_SHRINK", and the
`c->heap.root->next != NULL` term of should_gc in
memory_ensure_free_with_roots).
Code that references compound literals in nearly every function - Gleam-
and Elixir-compiled code in particular - therefore ran a full copying
collection every few instructions. Instrumenting a Gleam-compiled
interpreter running fib(15) showed 17,000 collections for 14M words of
allocation requests, ~6,000 of them forced purely by fragments and ~4,000
by the follow-up shrink; the run took 3.3s and spent 98% of it in
memory_scan_and_copy.
Fragments are ordinary heap memory: the collector already copies out of
them and frees the whole chain afterwards. So fold them in only when they
are large - more than a quarter of the young heap or 64k words
(memory_heap_fragments_need_gc) - and otherwise leave them for the next
natural collection. A running total of fragment words is kept on the Heap
so both this test and memory_heap_memory_size (evaluated on every
allocation under the fibonacci policy) are O(1) instead of walking the
chain, which became quadratic once fragments were allowed to accumulate.
With ~100k live words in the process, a loop touching one compound literal
per iteration (300k iterations) goes from >100s (bounded_free) / 62s
(fibonacci) to 140ms / 83ms; a loop allocating one boxed float per
iteration from 62s to 15ms under fibonacci growth. Under the default
bounded_free policy allocation-heavy loops remain slow for a different
reason (the heap is shrunk whenever free space exceeds 2*(need+16) words,
so it collects every few allocations); that is left as is here.
test-erlang, test-heap, test-mailbox, test-structs, test-enif and the
estdlib/eavmlib/alisp/etest suites pass.
Signed-off-by: Alistair Smith <hi@alistair.sh>
Both were `try maps:get/2 catch error:{badkey, _}`, so every lookup of a
missing key raised and caught an exception - and each raise builds a raw
stacktrace. Code that uses maps as dictionaries (Gleam's `dict.get`,
property lookups in an interpreter) misses constantly; in one profile
thousands of raises per second came from these two functions alone.
Use erlang:is_map_key/2 + erlang:map_get/2 instead, keeping the
`{badmap, Map}` error for non-map arguments (as OTP does).
Signed-off-by: Alistair Smith <hi@alistair.sh>
Contributor
|
Thank you for the profiling/token burn - believe GC will be much improved over in #2129 so unsure of that one - the other two looks interesting, will investigate them.. Afaik Paul has a lot of performance/GC improvements lined up when he is back from afk time. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Note
This pull request and most of the PR description was written by a language model. I would have liked to write it myself but currently do not have time to do so. I was not sure if Atom had an LLM policy, so thought it would at least be worth making the PR so maintainers can be aware that there may be potential performance savings to be made! I also have not looked at the code yet, either. Please feel free to close this as I understand your time is valuable and drive-by LLM prs can be very time wasting! Thank you for AtomVM ❤️
Summary
This PR fixes three separate hot paths I found while profiling a Gleam-compiled JS interpreter on AtomVM-WASM. It was running 100-1000x slower than the same code on the BEAM. None of the fixes are specific to that program.
caseon atoms is now a single bit compare. Atom comparison went throughterm_compare, which looks up both atom names and compares the strings which was 78% of profile samples. The fast path now just compares the raw terms and everything else still usesterm_compare.fib(15)in the interpreter caused 17,000 collections. Small fragments now just wait for the next normal GC. Large ones still collect early.maps:find/2andmaps:get/3no longer raise on every miss. They were try/catch aroundmaps:get/2, so every miss built a stacktrace. They now useis_map_keyandmap_get. Bonus fix:maps:find/2on a non-map now raises{badmap, Map}like OTP.Numbers
benchmark script bench_pr.erl.txt - 300k iterations each on macOS arm64:
select_valover 8 atomsbounded_free(default)fibonaccifibonaccibounded_freeIn the interpreter itself,
fib(15)went from 2888 ms to 16 ms, and the browser build went from 1600 ms to 22 ms.Testing
All test suites pass before and after except
test_epmdandtest_net_kernelwhich also fail on main. Each commit builds on its own and updates the CHANGELOG.* Why allocation-heavy loops stay slow under
bounded_free(not changed in this PR)Under
bounded_free, the heap shrinks whenever free space is more than about twice the live size. So a loop that allocates still collects every few allocations, with or without this PR. I left the policy alone. Happy to discuss whether it should be less eager, or whetherfibonaccishould be the default on non-embedded platforms.