Open
Conversation
Also use the aterm _strorage to effectively store integer terms.
There was a problem hiding this comment.
Pull request overview
This PR aims to compact the ATerm memory representation by removing per-term annotation storage from the general SharedTerm representation and introducing a dedicated integer-term storage path, with associated API/doc updates across the ATerm ecosystem.
Changes:
- Remove annotation handling from general term representations (
SharedTerm,Termtrait, and related macro-generated APIs), and shift integer value storage to a dedicatedSharedTermIntrepresentation. - Extend term storage with a separate integer-term table (
int_terms) and update global pool integer creation to use it. - Update allocator freelist internals and strengthen tests/documentation across term/symbol/pool modules.
Reviewed changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| crates/unsafety/src/block_allocator.rs | Switch freelist next to nullable raw pointer and improve tests for allocator reuse/aliasing. |
| crates/macros/src/merc_derive_terms.rs | Remove generated annotation() from derived term trait delegations. |
| crates/data/src/data_expression.rs | Update machine-number extraction to go through ATermIntRef. |
| crates/aterm/src/symbol.rs | Doc/comment cleanups and link updates. |
| crates/aterm/src/storage/thread_aterm_pool.rs | Doc updates around term construction and thread pool role. |
| crates/aterm/src/storage/symbol_pool.rs | Doc/link tweaks for symbol pool. |
| crates/aterm/src/storage/shared_term.rs | Remove annotation storage from SharedTerm; adjust layout/equality/hash/debug. |
| crates/aterm/src/storage/mod.rs | Module-level doc link correction. |
| crates/aterm/src/storage/global_aterm_pool.rs | Route integer-term creation through new insert_int_term path. |
| crates/aterm/src/storage/gc_mutex.rs | Documentation clarifications for GC guard behavior. |
| crates/aterm/src/storage/aterm_storage.rs | Add separate int_terms storage and SharedTermInt representation + insertion API. |
| crates/aterm/src/protected.rs | Documentation clarifications for protection guards. |
| crates/aterm/src/markable.rs | Documentation updates and typo fix. |
| crates/aterm/src/aterm_list.rs | Documentation tweaks + remove annotation() from generated term traits. |
| crates/aterm/src/aterm_int.rs | Rework integer value access to read from SharedTermInt via pointer cast. |
| crates/aterm/src/aterm_builder.rs | Spelling fix in docs. |
| crates/aterm/src/aterm_binary_stream.rs | Documentation link/name corrections. |
| crates/aterm/src/aterm.rs | Remove annotation() from Term trait and implementations; doc improvements. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 21 out of 21 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
1975a5c to
665bf8e
Compare
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
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.
Tasks
ATermInt.SharedTermto to remove 1 word perATermRef.SharedTerm.As a side effect this will also allow loop unrolling the equals, and hash functions. Also, figure out why the lookup benchmark is now significantly slower, it seems unlikely that this is just because of performance differences and instead it seems that the benchmark might be wrong.