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

Implement chain index #855

Merged
merged 8 commits into from
Nov 19, 2020
Merged

Implement chain index #855

merged 8 commits into from
Nov 19, 2020

Conversation

austinabell
Copy link
Contributor

Summary of changes
Changes introduced in this pull request:

  • sparse cache of tipsets for quicker lookback tipset retrievals Fix Tipindex and implement Tipset caching #814
    • I decided to just match Lotus' naming for convenience
  • Cleans up mutex usage to avoid holding locks longer than needed
  • Removes mutex from state tree (overhead is not needed since it's only used in sync contexts)

Reference issue to close (if applicable)

Closes #814

Other information and links

loop {
let pts = self.load_tipset(ts.parents()).await?;

if to > pts.epoch() {
Copy link
Member

Choose a reason for hiding this comment

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

I'm guessing this to detect for empty epochs? Had to stare at that for a hot sec to see why this would ever happen lol

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah exactly haha

where
BS: BlockStore + Send + Sync + 'static,
{
if let Some(ts) = cache.write().await.get(tsk) {

Choose a reason for hiding this comment

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

why not use cache.read() here instead of getting a write lock?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because it's a least recently used (LRU) cache, and it needs to update the position of the entry on read to function correctly, which requires mutable access. There is a method called peek on this cache, which only requires immutable access, but that defeats the purpose of this cache.

Yes this limits the concurrent reads, but the lock is kept for a very short amount of time and this means that frequently accessed keys don't get evicted from the cache early

@austinabell austinabell merged commit 2608270 into main Nov 19, 2020
@austinabell austinabell deleted the austin/tscache branch November 19, 2020 20:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fix Tipindex and implement Tipset caching
4 participants