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

Internal structs for ThreadAware AccountRead/WriteLocks #31431

Conversation

apfitzge
Copy link
Contributor

@apfitzge apfitzge commented May 1, 2023

Problem

Running an experiment w/ unlocking threads which requires using a DashMap inside the thread aware locks struct.
The interface for DashMap is slightly different, and we cannot do in-place tuple destructuring like is currently done; The code ended up having .0, or .1 in several places which feels very brittle and less readable.

Summary of Changes

Create simple structs to hold Account-specific read and write locks w/ named fields.
This makes changes (described above) cleaner if we end up doing that.

Fixes #

@apfitzge
Copy link
Contributor Author

apfitzge commented May 1, 2023

For reference, w/ this PR changing to using DashMaps becomes significantly simpler: 73bdfa2

Where the changes are limited to imports, types, and removing mutability.

Not sure the experiment will pan out, but think the change in this PR is good regardless.

@codecov
Copy link

codecov bot commented May 1, 2023

Codecov Report

Merging #31431 (3b71f4f) into master (4b0e16d) will increase coverage by 0.0%.
The diff coverage is 100.0%.

@@           Coverage Diff           @@
##           master   #31431   +/-   ##
=======================================
  Coverage    81.5%    81.5%           
=======================================
  Files         734      734           
  Lines      207159   207171   +12     
=======================================
+ Hits       168959   168988   +29     
+ Misses      38200    38183   -17     

@apfitzge apfitzge marked this pull request as ready for review May 2, 2023 23:45
@apfitzge apfitzge requested review from ryoqun and tao-stones May 2, 2023 23:45
Copy link
Contributor

@tao-stones tao-stones left a comment

Choose a reason for hiding this comment

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

lgtm

@apfitzge apfitzge merged commit 886aea2 into solana-labs:master May 4, 2023
@apfitzge apfitzge deleted the feature/thread_aware_account_locks_internal_types branch May 4, 2023 16:38
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.

2 participants