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

Overhaul internals to support faster execution #111

Merged
merged 20 commits into from
Mar 23, 2020
Merged

Overhaul internals to support faster execution #111

merged 20 commits into from
Mar 23, 2020

Conversation

carllerche
Copy link
Member

The internals are overhauled to avoid as much allocation as possible.
This is done by leveraging the object store as much as possible and hard
coding settings such as max threads. Additionally, the history tracked
by atomic cells is limited. These changes enable tracking state without
the use of vecs and other allocated collections.

The "backtrace" feature is removed in favor of the nightly Location
API. To enable this, a nightly Rust must be used and --cfg loom_nightly specified at compile time.

This patch comes with a number of breaking changes, including:

  • CausalCell deferred checks is removed as doing so is undefined
    behavior.
  • CausalCell is renamed to UnsafeCell.

The internals are overhauled to avoid as much allocation as possible.
This is done by leveraging the object store as much as possible and hard
coding settings such as max threads. Additionally, the history tracked
by atomic cells is limited. These changes enable tracking state without
the use of vecs and other allocated collections.

The "backtrace" feature is removed in favor of the nightly `Location`
API. To enable this, a nightly Rust must be used and `--cfg
loom_nightly` specified at compile time.

This patch comes with a number of breaking changes, including:

* `CausalCell` deferred checks is removed as doing so is undefined
  behavior.
* `CausalCell` is renamed to `UnsafeCell`.
@carllerche
Copy link
Member Author

@hawkw This should be ready to review / merge.

@carllerche
Copy link
Member Author

Running this command against the Tokio repo:

time LOOM_MAX_PREEMPTIONS=3 RUSTFLAGS="--cfg loom" cargo test --lib --all-features --release -- loom_list --nocapture

Master: 631,933 iterations

15.08s user 0.53s system 99% cpu 15.643 total

This PR: 231,145 iterations

2.06s user 0.04s system 99% cpu 2.113 total

Copy link
Member

@hawkw hawkw left a comment

Choose a reason for hiding this comment

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

The new location tracking is very cool, and I'm excited to get to use it! Bounding the history and using stack arrays seems like a great, simple way to improve loom performance. I'd like to take a closer look at the changes to causality rules, but I've left a few minor comments for now.

src/cell/unsafe_cell.rs Show resolved Hide resolved
src/cell/unsafe_cell.rs Show resolved Hide resolved
src/rt/arc.rs Show resolved Hide resolved
src/rt/atomic.rs Show resolved Hide resolved
src/rt/atomic.rs Show resolved Hide resolved
src/rt/atomic.rs Outdated Show resolved Hide resolved
src/rt/location.rs Show resolved Hide resolved
Copy link
Member

@hawkw hawkw left a comment

Choose a reason for hiding this comment

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

this looks really great --- and i appreciate the number of comments in the new code, made reviewing much easier. i left a few more minor comments.

src/rt/mod.rs Show resolved Hide resolved
src/rt/object.rs Show resolved Hide resolved
src/rt/object.rs Outdated Show resolved Hide resolved
src/rt/object.rs Outdated Show resolved Hide resolved
src/rt/object.rs Show resolved Hide resolved
src/rt/path.rs Outdated Show resolved Hide resolved
src/rt/path.rs Outdated Show resolved Hide resolved
src/rt/path.rs Outdated Show resolved Hide resolved
src/rt/atomic.rs Outdated Show resolved Hide resolved
src/rt/atomic.rs Show resolved Hide resolved
carllerche and others added 9 commits March 23, 2020 14:09
Co-Authored-By: Eliza Weisman <eliza@buoyant.io>
Co-Authored-By: Eliza Weisman <eliza@buoyant.io>
Co-Authored-By: Eliza Weisman <eliza@buoyant.io>
Co-Authored-By: Eliza Weisman <eliza@buoyant.io>
@carllerche carllerche merged commit 88e4225 into master Mar 23, 2020
@seanmonstar seanmonstar deleted the exp branch March 24, 2020 18:27
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