Skip to content
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

Integrate PMA #143

Merged
merged 132 commits into from
Dec 20, 2023
Merged

Integrate PMA #143

merged 132 commits into from
Dec 20, 2023

Conversation

eamsden
Copy link
Collaborator

@eamsden eamsden commented Nov 29, 2023

Integrate the PMA work done by @barter-simsum and add logic to stash nouns and cold state in it.

goals for this PR

We intend to integrate this PR once the following are possible:

  • booting to a dojo prompt
  • not crashing on sync()
  • restoring the pier state and continuing with an interactive dojo prompt

Further issues will be opened for bugfixes and TODOs, but this will allow us to integrate codegen and other pending work against a stable interface.

archived TODO

Edward TODO

  • High-level rust bindings
  • composable Persist trait
  • tie PMA into serf
  • strip out old PMA and double-jam code
  • Change Hamt to be pointer-to-stem instead of stem. (It should have been all along, and this facilitates handle_from_u64)
  • Implement Persist for Noun
  • Implement Persist for Hamt 😓
  • Implement Persist for Batteries
  • Implement Persist for BatteriesList
  • Implement Persist for NounList
  • Implement Persist for Cold
  • Make unifying equality dirty things in the PMA

(after Alex's review)

  • Refactor save logic and move into serf
  • XX comments about where to put warm and hot state so they are not blown away by stack resets
  • Remove StemTraversalEntry
  • Fix incomplete and out-of-date comments
  • Make Persist for Noun use Persist for Atom implementation for atom case
  • Push cell children onto stack in copy_to_buffer for Noun
  • Remove unnecessary continues in HAMT persist loops
  • Make named pointer local vs handle destructuring consistent in Persist instances
  • Heavy-duty doc-comment on the implementation of the Preserve trait and the phase-separation between size-counting and actual copying.
  • Heavy-duty doc-comment on the representation of HAMTs

David TODO

  • add incremented txn id logic to bt_sync
  • write routine to restore memory maps (became obvious after implementing _bt_data_cow
  • check return of all mmap calls to not be MAP_FAILED - abort if so -- actually should check that the return is the
    fixed addr passed in since mmap makes no guarantee that it will map at that address, but we rely upon that
  • complete implementation of bt_state_close() - got neglected
  • c test to verify integrity of btree on a bt_state_close() bt_state_open() cycle
  • more thorough c tests to validate _bt_insert that will exercise deletion coalescing
  • bt_malloc test - validate modifications to mlist, flist, and possibly btree though that may be adequately covered by _bt_insert
  • bt_free test. validate modifications to mlist and possibly btree
  • possibly a test that will combine enough bt_malloc and bt_free calls and follow that up with a call to bt_sync to cover a scenario that will drop at least one tree
  • remove CAN_COALESCE macro defn and usage. btree deletion coalescing implemented
  • remove c-side warnings generated during cargo build

Alex TODO

  • PMA code reviewed (C-side)
  • PMA integration code reviewed (Rust-side)

@@ -51,6 +50,7 @@ pub struct NockStack {
alloc_pointer: *mut u64,
/** MMap which must be kept alive as long as this NockStack is */
memory: MmapMut,
/** PMA from which we will copy into the NockStack */
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Incoming member that's not yet present?

rust/ares/src/mem.rs Show resolved Hide resolved
rust/ares/src/persist.rs Show resolved Hide resolved
@eamsden
Copy link
Collaborator Author

eamsden commented Dec 16, 2023

@ashelkovnykov @joemfb the plan is to merge this once it is capable of

  • booting to a dojo prompt
  • not crashing on sync()
  • restoring the pier state and continuing with an interactive dojo prompt

Please review with an eye toward this merge. Bugs, TODOs, and cleanup which are not on the path to merging the above functionality with a CI greencheck are to be filed as issues and worked on in separate PRs.

I believe the interface for this is sufficiently stabilized that merging it will enable me to focus on integrating other components, most notably codegen.

@eamsden eamsden marked this pull request as ready for review December 20, 2023 05:49
Copy link
Contributor

@ashelkovnykov ashelkovnykov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand 100% of the Rust changes and 15% of the C changes - shipping a review to get this out of monolithic status.

@eamsden eamsden merged commit c6a45ae into status Dec 20, 2023
1 check passed
@eamsden eamsden deleted the eamsden/integrate-pma branch December 20, 2023 15:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants