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.

Sign up for GitHub

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

Jump to bottom

WIP feat: use pattern-based compression #6432

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
Draft

WIP feat: use pattern-based compression #6432

wants to merge 2 commits into from
+50 −20

Conversation

erights
Copy link
Member

@erights erights commented Oct 10, 2022 •
edited
Loading

#endo-branch: markm-pattern-based-compression-2

Won't work until updated to an endo incorporating endojs/endo#1584

Fixes #3167

Modified SwingSet's virtual object stores so that if they have a stateShape, they also use it for compression.

Modified SwingSet's virtual mapStores so that if they have a valueShape option, they also use it for compression.

Modified ERTP's paymentLedger to provide the amountShape as the valueShape option for the durable ledger mapStore.

Verified in the debugger that we now only store a singleton array around the value itself, dropping the enclosing amount structure and the brand which is always the same for a given ledger mapStore. IOW, instead of storing the serialization of

{
   brand: moolaBrand,
   value: 2n,
}

for every live payment, with this PR we store the serialization of

[2n]

for every live payment. This is not just a savings because it is smaller. We also avoid needing to adjust the refcount of their common moolaBrand.

Once we've switched to smallcaps, the serialization of [2n] is tiny.

After rebasing on #6431 , I verified in the debugger that invitations benefit additionally from InvitationElementShape. Will be nice to look at once further combined with smallcaps.

@dtribble @turadg @FUDCo @gibson042 @mhofman when it is ready I'll ask you to review. Until then please ignore.

@erights erights self-assigned this Oct 10, 2022
@erights erights changed the base branch from master to markm-split-reform October 10, 2022 20:47
@erights erights marked this pull request as draft October 10, 2022 20:59
@erights erights marked this pull request as ready for review October 10, 2022 21:54
@erights erights changed the base branch from markm-split-reform to markm-pattern-compress October 10, 2022 23:10
@erights erights changed the base branch from markm-pattern-compress to markm-split-reform October 10, 2022 23:27
@erights erights changed the title fix: compress with split reform fix: pattern-based compress with split reform Oct 10, 2022
@erights erights mentioned this pull request Oct 10, 2022
Closed
@erights erights requested a review from FUDCo October 10, 2022 23:32
@erights erights force-pushed the markm-compress-with-split-reform branch from a7e2b86 to b510e73 Compare October 10, 2022 23:35
@erights erights force-pushed the markm-compress-with-split-reform branch from b510e73 to b83b479 Compare October 11, 2022 04:20
@erights erights changed the title fix: pattern-based compress with split reform fix: pattern-based compression with split reform Oct 11, 2022
@erights erights mentioned this pull request Oct 11, 2022
Open
@erights erights requested a review from mhofman October 11, 2022 23:52
@erights erights mentioned this pull request Oct 13, 2022
Open
@erights erights force-pushed the markm-compress-with-split-reform branch from b7312bb to f966689 Compare October 19, 2022 02:20
@erights erights force-pushed the markm-compress-with-split-reform branch from f966689 to 7e458fe Compare October 20, 2022 07:50
@erights erights force-pushed the markm-compress-with-split-reform branch from 7e458fe to f77aa27 Compare November 5, 2022 03:22
@erights erights mentioned this pull request Nov 5, 2022
Merged
@erights erights marked this pull request as draft November 5, 2022 03:52
@erights erights mentioned this pull request Apr 14, 2024
Closed
@erights
Copy link
Member Author

erights commented Apr 14, 2024

See #9232 re CI failure

@michaelfig michaelfig force-pushed the markm-compress-with-split-reform branch 2 times, most recently from 94ae720 to 010f2bc Compare April 15, 2024 19:45
@michaelfig michaelfig mentioned this pull request Apr 15, 2024
Merged
@michaelfig michaelfig force-pushed the markm-compress-with-split-reform branch from 010f2bc to 0fe4755 Compare April 15, 2024 20:23
@erights erights force-pushed the markm-compress-with-split-reform branch from 0fe4755 to ab10583 Compare April 16, 2024 22:23
mergify bot added a commit that referenced this pull request Apr 19, 2024
@mergify
ci(restore-node): preserve package.json timestamp (#9237)
a55bc1e 
<!-- < < < < < < < < < < < < < < < < < < < < < < < < < < < < < < < < < ☺
v                               ✰  Thanks for creating a PR! ✰
☺ > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > >  -->

<!-- Most PRs should close a specific Issue. All PRs should at least
reference one or more Issues. Edit and/or delete the following lines as
appropriate (note: you don't need both `refs` and `closes` for the same
one): -->

closes: #9232
refs: #6432

## Description

The `restore-node` Github action was updating the timestamp on
`package.json` if there was an `#endo-branch: XXX` setting on the PR.
This caused `agd-builder.sh` to rebuild the cached build artifacts
unnecessarily, including using incorrect `@endo/*` package versions (the
ones in the PR's `package.json`s, not the ones from branch `XXX`).

Now the `package.json` is copied (preserving its timestamp), and moved
back into place after installing any replaced packages.

<!-- Add a description of the changes that this PR introduces and the
files that
are the most critical to review.
-->

### Security Considerations

n/a

<!-- Does this change introduce new assumptions or dependencies that, if
violated, could introduce security vulnerabilities? How does this PR
change the boundaries between mutually-suspicious components? What new
authorities are introduced by this change, perhaps by new API calls?
-->

### Scaling Considerations

With this PR, shorter CI runs.

<!-- Does this change require or encourage significant increase in
consumption of CPU cycles, RAM, on-chain storage, message exchanges, or
other scarce resources? If so, can that be prevented or mitigated? -->

### Documentation Considerations

n/a

<!-- Give our docs folks some hints about what needs to be described to
downstream users.

Backwards compatibility: what happens to existing data or deployments
when this code is shipped? Do we need to instruct users to do something
to upgrade their saved data? If there is no upgrade path possible, how
bad will that be for users?

-->

### Testing Considerations

n/a

<!-- Every PR should of course come with tests of its own functionality.
What additional tests are still needed beyond those unit tests? How does
this affect CI, other test automation, or the testnet?
-->

### Upgrade Considerations

n/a

<!-- What aspects of this PR are relevant to upgrading live production
systems, and how should they be addressed? -->
@michaelfig
Copy link
Member

@erights, it looks like CI is successful now (modulo Cloudflare pages build). I think you can continue with testing and resolving conflicts here.

@erights erights force-pushed the markm-compress-with-split-reform branch 3 times, most recently from 5df92df to 2904c1c Compare April 29, 2024 02:28
@erights erights mentioned this pull request Apr 29, 2024
Merged
erights added a commit to endojs/endo that referenced this pull request Apr 29, 2024
@erights
refactor(patterns): improve pattern types. Flatten @imports (#2256)
258dfc6 
closes: #XXXX
refs: #2248 #1584 Agoric/agoric-sdk#6432

## Description

Pure refactor. Changes only static info. Mostly more consistent and more
readable use of `@import`.

One case made less readable: Remove newlines within a large `@import`
directive. The reason is that
`yarn lerna run build:types` chokes on those newlines. TODO minimal
repro + report issue.

Extracted from other PRs #1584 #2248 which are now staged on this one.
But this should be a reviewable and mergeable improvement regardless of
whether we move forward on the others.

### Security Considerations

none
### Scaling Considerations

none
### Documentation Considerations

none
### Testing Considerations

none
### Compatibility Considerations

none
### Upgrade Considerations
none

- ~[ ] Includes `*BREAKING*:` in the commit message with migration
instructions for any breaking change.~
- ~[ ] Updates `NEWS.md` for user-facing changes.~
@erights erights force-pushed the markm-compress-with-split-reform branch from 2904c1c to 47cc7d5 Compare April 29, 2024 21:48
@erights erights force-pushed the markm-compress-with-split-reform branch 2 times, most recently from b24cae0 to 53dba29 Compare May 9, 2024 00:16
@erights erights force-pushed the markm-compress-with-split-reform branch from dfae2c8 to f417938 Compare June 2, 2024 19:32
@erights erights force-pushed the markm-compress-with-split-reform branch 2 times, most recently from af411ac to e0e3932 Compare June 13, 2024 14:09
@erights erights force-pushed the markm-compress-with-split-reform branch from e0e3932 to 618971a Compare June 22, 2024 04:01
@erights erights force-pushed the markm-compress-with-split-reform branch from 618971a to 634bd34 Compare July 3, 2024 00:29
@erights erights force-pushed the markm-compress-with-split-reform branch 2 times, most recently from f9c5d02 to 80f5d0c Compare August 5, 2024 00:10
@erights erights force-pushed the markm-compress-with-split-reform branch 3 times, most recently from ff230fb to e9c28f6 Compare September 7, 2024 20:50
@erights erights force-pushed the markm-compress-with-split-reform branch from e9c28f6 to 79040b4 Compare October 14, 2024 20:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@erights erights

Labels
patterns
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

ERTP: payment ledger can just store values
2 participants
@erights @michaelfig