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

UIP-0103: Persistent Nock Caching + Loom Migration Framework + Pointer Compression Migration Refactor #508

Merged
merged 5 commits into from
Nov 8, 2023

Conversation

matthew-levan
Copy link
Contributor

@matthew-levan matthew-levan commented Sep 1, 2023

Implements UIP-0103, establishes a pattern for implementing loom migrations, and refactors the pointer compression as a compile-time feature instead of a runtime one.

See #496 for original work on UIP-0103.

TODO:

Tested working in all vanes I tested (Iris, etc. are a little more cumbersome):

  • Ames
  • Arvo
  • Behn
  • Clay
  • Dill
  • Eyre
  • Gall

@matthew-levan matthew-levan added the feature New feature or feature request label Sep 1, 2023
@matthew-levan matthew-levan changed the title Loom migration "framework" (with pointer compression migration refactor) Loom migration framework (with pointer compression migration refactor) Sep 6, 2023
@matthew-levan matthew-levan force-pushed the msl/loom-migration-refactor branch 2 times, most recently from f35cd83 to 8570654 Compare September 8, 2023 19:39
@matthew-levan matthew-levan marked this pull request as ready for review September 11, 2023 15:13
@matthew-levan matthew-levan requested a review from a team as a code owner September 11, 2023 15:13
@matthew-levan
Copy link
Contributor Author

Notes from @barter-simsum:

  • Consider defining a macro to generate versioned symbols, and rename u3j_vX..., etc. as _mig_vX...

Example of a versioned macro:

#define VERSIONED(x) _mig_v ## x
VERSIONED(_cj_free_hank) (params) { defn }

@matthew-levan matthew-levan force-pushed the msl/loom-migration-refactor branch 2 times, most recently from 69f75b9 to 4cc9d1e Compare September 15, 2023 16:57
pkg/noun/allocate.c Outdated Show resolved Hide resolved
pkg/noun/jets.c Outdated Show resolved Hide resolved
pkg/noun/jets.h Outdated Show resolved Hide resolved
pkg/noun/nock.h Outdated Show resolved Hide resolved
pkg/noun/v1/jets.h Outdated Show resolved Hide resolved
pkg/noun/v1/allocate.h Outdated Show resolved Hide resolved
pkg/noun/v1/nock.c Outdated Show resolved Hide resolved
pkg/noun/v1/nock.h Outdated Show resolved Hide resolved
@matthew-levan matthew-levan force-pushed the msl/loom-migration-refactor branch 2 times, most recently from 6534adc to ecb803a Compare September 15, 2023 20:36
@matthew-levan matthew-levan changed the title Loom migration framework (with pointer compression migration refactor) UIP-0103: Persistent Nock Caching + Loom Migration Framework + Pointer Compression Migration Refactor Sep 19, 2023
pkg/noun/allocate.h Outdated Show resolved Hide resolved
pkg/noun/manage.c Outdated Show resolved Hide resolved
pkg/noun/allocate.h Outdated Show resolved Hide resolved
pkg/noun/manage.h Outdated Show resolved Hide resolved
pkg/noun/options.h Show resolved Hide resolved
pkg/noun/v2/manage.c Outdated Show resolved Hide resolved
pkg/noun/v2/manage.c Outdated Show resolved Hide resolved
pkg/noun/v3/manage.c Show resolved Hide resolved
pkg/noun/v3/manage.c Outdated Show resolved Hide resolved
pkg/noun/v3/vortex.h Show resolved Hide resolved
pkg/noun/v2/nock.h Outdated Show resolved Hide resolved
pkova added a commit that referenced this pull request Nov 8, 2023
This PR adjusts the serf's memory-pressure thresholds, adds state to
track threshold events and avoid thrashing ("schmitt trigger"), and
refactors post-event operation to use a bitmap and track operations more
granularly.

These changes are useful on there own, and necessary for adding
memory-pressure-based trimming to #508.
Copy link
Member

@joemfb joemfb left a comment

Choose a reason for hiding this comment

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

this will do

@matthew-levan matthew-levan merged commit 87783a9 into develop Nov 8, 2023
5 checks passed
@matthew-levan matthew-levan deleted the msl/loom-migration-refactor branch November 8, 2023 16:36
pkova added a commit that referenced this pull request Nov 10, 2023
This PR dedicates slot 0 to cells (`u3a_minimum`), and aligns subsequent
slots on power-of-two boundaries. Previously, slot 0 was unused, and
subsequent slots were aligned *near* power-of-two boundaries. For
example:

| words | old slot | new slot |
| --- | - | - |
|   6 | 1 | 0 |
|  31 | 3 | 2 |
|  61 | 4 | 3 |
|  62 | 4 | 3 |
|  63 | 4 | 3 |
| 121 | 5 | 4 |
| 122 | 5 | 4 |
| 123 | 5 | 4 |
| 124 | 5 | 4 |
| 125 | 5 | 4 |
| 126 | 5 | 4 |
| 127 | 5 | 4 |

The first issue dates to urbit/urbit#987. Slot 0 was reserved for sizes
less than 8, which meant that 7-word allocations on the home road were
incredibly slow (as they traversed a free list full of 6 word
allocations). The second issue has always been the case in this
allocator, due to the way that the size was rounded up on each iteration
of the "power-of-two" sizing loop.

This PR does not address the infamous size-bumping logic in the
allocator:


https://github.com/urbit/vere/blob/9bdc1af00f6650e27ac66793c6781d933dd30dfe/pkg/noun/allocate.c#L453-L467

I'm been tempted to disable that behavior on the home-road (to further
reduce fragmentation), but I'm not convinced that won't still run into
pathological performance issues. The best approach might be to start
searching the "proper" free list, but bound the number of iterations
before bumping. Either change can be made at any time, without
migrations.

This PR does requires a migration to move free space into the
now-appropriate free-list. It includes a trivial, always on migration,
which should be refactored into #508 when appropriate.

This PR helps somewhat with urbit/urbit#6805, as it reduces home-road
fragmentation.
pkova added a commit that referenced this pull request Nov 10, 2023
This PR builds on #546, extending it's more-granular state management to
the persistent memoization cache introduced in #508
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature or feature request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants