Skip to content
This repository has been archived by the owner on Nov 6, 2020. It is now read-only.

Implement len caching for parking_lot RwLock #10032

Merged
merged 12 commits into from
Dec 13, 2018
Merged

Implement len caching for parking_lot RwLock #10032

merged 12 commits into from
Dec 13, 2018

Conversation

mattrutherford
Copy link
Contributor

  • Rename crate to len-caching-lock
  • Implement for parking_lot::RwLock
  • Implement Len trait for all Rust std::collections
  • Improve docs and tests

Required for a different ongoing WIP PR

Closes #10026

@parity-cla-bot
Copy link

It looks like @mattrutherford signed our Contributor License Agreement. 👍

Many thanks,

Parity Technologies CLA Bot

@mattrutherford mattrutherford added the A0-pleasereview 🤓 Pull request needs code review. label Dec 7, 2018
@5chdn 5chdn added the M4-core ⛓ Core client code / Rust. label Dec 9, 2018
@5chdn 5chdn added this to the 2.3 milestone Dec 9, 2018
andresilva
andresilva approved these changes Dec 10, 2018
View reviewed changes
Copy link
Contributor

@andresilva andresilva left a comment

Choose a reason for hiding this comment

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

minor whitespace nits

util/len-caching-lock/src/mutex.rs Outdated Show resolved Hide resolved
util/len-caching-lock/src/rwlock.rs Outdated Show resolved Hide resolved
util/len-caching-lock/src/rwlock.rs Outdated Show resolved Hide resolved
@mattrutherford
Copy link
Contributor Author

mattrutherford commented Dec 10, 2018 •
edited
Loading

Thanks @andresilva - now done, plus one other small thing I noticed.

tomusdrw
tomusdrw reviewed Dec 10, 2018
View reviewed changes
Copy link
Collaborator

@tomusdrw tomusdrw left a comment

Choose a reason for hiding this comment

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

Looks good! Few potential improvements.

util/len-caching-lock/src/lib.rs Outdated Show resolved Hide resolved
util/len-caching-lock/src/mutex.rs Show resolved Hide resolved
util/len-caching-lock/src/mutex.rs Outdated Show resolved Hide resolved
util/len-caching-lock/src/rwlock.rs Outdated Show resolved Hide resolved
util/len-caching-lock/src/rwlock.rs Show resolved Hide resolved
@mattrutherford mattrutherford force-pushed the mr-10026-parking-lot-len-cache branch from 5d7b350 to b80ad95 Compare December 11, 2018 12:36
tomusdrw
tomusdrw approved these changes Dec 13, 2018
View reviewed changes
Copy link
Collaborator

@tomusdrw tomusdrw left a comment

Choose a reason for hiding this comment

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

Awesome! 🎉

@tomusdrw tomusdrw added A7-looksgoodcantmerge 🙄 Pull request is reviewed well, but cannot be merged due to conflicts. and removed A0-pleasereview 🤓 Pull request needs code review. labels Dec 13, 2018
@mattrutherford
Copy link
Contributor Author

Thanks for the review and comments @tomusdrw !

@mattrutherford mattrutherford force-pushed the mr-10026-parking-lot-len-cache branch from e685910 to e90432f Compare December 13, 2018 16:57
@mattrutherford mattrutherford removed the A7-looksgoodcantmerge 🙄 Pull request is reviewed well, but cannot be merged due to conflicts. label Dec 13, 2018
@mattrutherford mattrutherford merged commit 6071822 into master Dec 13, 2018
@mattrutherford mattrutherford deleted the mr-10026-parking-lot-len-cache branch December 13, 2018 18:08
niklasad1 pushed a commit that referenced this pull request Dec 16, 2018
@mattrutherford @niklasad1
Implement len caching for parking_lot RwLock (#10032)
fcf7349 
- Refactor (and rename crate) and implement RwLock len cache.

- improve docs

- update ethcore verification queue to new version

- Implement Default, From,  also derive Debug

- update: parking_lot to 0.7
@5chdn 5chdn added the A8-looksgood 🦄 Pull request is reviewed well. label Jan 2, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@tomusdrw tomusdrw tomusdrw approved these changes

@andresilva andresilva andresilva approved these changes

Assignees
No one assigned
Labels
A8-looksgood 🦄 Pull request is reviewed well. M4-core ⛓ Core client code / Rust.
Projects
None yet
Milestone
2.3
Development

Successfully merging this pull request may close these issues.
5 participants
@mattrutherford @parity-cla-bot @andresilva @tomusdrw @5chdn