fix: replace per-row __id__ clauses with Map lookup#68
Merged
giustinoesposito-prima merged 2 commits intomasterfrom Feb 23, 2026
Merged
fix: replace per-row __id__ clauses with Map lookup#68giustinoesposito-prima merged 2 commits intomasterfrom
giustinoesposito-prima merged 2 commits intomasterfrom
Conversation
The ARM64 JIT backend in OTP 25 (erts-13.2) silently generates
incorrect machine code when a function has more than 4096 clauses
of integer pattern matching, causing all calls to fall through to
the catch-all clause and return nil.
Previously, `gen_internal_id/3` generated one `def __id__(N)` clause
per CSV row plus a catch-all. For large CSVs (e.g. comuni.csv with
7896 rows) this exceeded the 4096-clause threshold, breaking lookups
on ARM64 (Apple Silicon via Docker).
Replace the N individual clauses with a single `@__csv_map__` module
attribute storing a `%{id => changeset}` map, and a single
`def __id__/1` that performs a Map.fetch/2 lookup.
giustinoesposito-prima
requested review from
eFFeeMMe and
opsidao
and removed request for
a team
claudio-dalicandro
requested changes
Feb 23, 2026
There was a problem hiding this comment.
For me the VM optimization compared to a Map lookup, which is O(n) anyway, is not relevant in this context. It is true that bumping the OTP version would be enough, and we should do that regardless, but I would go ahead with this change once the Credo violation is fixed, so we can immediately address a bug that is quite hard to detect (Kudos!!).
Co-authored-by: Claudio D'Alicandro <claudio.dalicandro@gmail.com>
claudio-dalicandro
approved these changes
Feb 23, 2026
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.
The ARM64 JIT backend in OTP 25 (erts-13.2) silently generates incorrect machine code when a function has more than 4096 clauses of integer pattern matching, causing all calls to fall through to the catch-all clause and return nil.
Previously,
gen_internal_id/3generated onedef __id__(N)clause per CSV row plus a catch-all. For large CSVs (e.g. comuni.csv with 7896 rows) this exceeded the 4096-clause threshold, breaking lookups on ARM64 (Apple Silicon via Docker).Replace the N individual clauses with a single
@__csv_map__module attribute storing a%{id => changeset}map, and a singledef __id__/1that performs a Map.fetch/2 lookup.