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

Improve ADT diffing logic used to track actor state chages #504

Closed
6 tasks
frrist opened this issue May 5, 2021 · 4 comments
Closed
6 tasks

Improve ADT diffing logic used to track actor state chages #504

frrist opened this issue May 5, 2021 · 4 comments
Labels
status/discussion Issue is under discussion

Comments

@frrist
Copy link
Member

frrist commented May 5, 2021

Description

Currently, Visor uses HAMT & AMT Diffing logic implemented in Lotus to determine what changed within actor states. The ADT diffing logic all lives here. There are two performance-related issues I see with the current diffing logic:

  1. It requires the state contained within the ADT to be deserialized to perform the diff.
  2. It will iterate over each entry in the ADT regardless if there has been a change or not. Needless serialization can be avoided by comparing the ADT Root CID before performing the Diff. Unfortunately, the ADT Root methods require a write to their underlying blockstore, which is not allowed in the current lotus implementation. I've described the issue in greater detail here
    • This also carries the side-effect of making the actor interfaces presented by lotus unimplementable due to their unexported interface methods.

ADT diffing is one of the largest bottlenecks of Visors chain state processing and must be improved for Visor to watch and process the chain in real-time.

We can address (1) in two parts:

  1. When diffing AMT's, use faster AMT diffing logic. code here.
  2. Implement and use fast HAMT diffing logic. issue here.

Addressing (2) requires some decisions/discussion. A couple of ideas on how to address this (open to more):

  1. Modify the ADT Root method to avoid writing to their underlying blockstore. Unclear if this is even possible.
  2. Make the blockstore used by lotus for ADT diffing writeable. Probably a bad idea since diffing should be read-only
  3. Extract the actor interfaces and diffing logic from lotus to a new repository or into Visor. This will allow the ADT to be backed by a blockstore that allows Root to be called. This will also make landing code changes easier as we won't be tied to the lotus review cycle but will become a repo we need to maintain as new actor versions are added.

Acceptance Criteria

@frrist frrist added the status/discussion Issue is under discussion label May 5, 2021
@iand
Copy link
Contributor

iand commented May 6, 2021

Is there an option 4: use the native actor states and not the lotus style actor interfaces

@frrist
Copy link
Member Author

frrist commented May 6, 2021

I think there is a lot over overlap between option 3 and 4. I assume we'd end up implementing something similar to the lotus shims if we choose to operate over the native actor states.

The largest drawback I see with this aproach is it adds more code for us to maintain and could slow out ability to release new versions of Visor when there is a network/specs-actors upgrade since we will need to update our shims.

@frrist
Copy link
Member Author

frrist commented May 6, 2021

After some synchronous discussion, we're left with a couple of options:

  1. Modify the HAMT (usage in specs-actors) and AMT (go-amt-ipld) code to work with a read-only IPLD store. We need to understand:
    a. Who would own this work?
    b. What would it require to implement?

  2. Implement visor-specific actor shim. This will sidestep the read-only store requirement but will add more code to maintain and update when there is a specs-actors release.

@frrist
Copy link
Member Author

frrist commented May 27, 2021

via #519 and #530

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status/discussion Issue is under discussion
Projects
None yet
Development

No branches or pull requests

2 participants