Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign upHoist bloom filter into scoped TLS, and remove a bunch of complexity and unsafety from the style system #14662
Conversation
highfive
commented
Dec 21, 2016
|
Heads up! This PR modifies the following files:
|
highfive
commented
Dec 21, 2016
|
|
|
@bors-servo try |
|
|
Hoist bloom filter into scoped TLS and simplify code I'm working on more patches to clean up the types, but might as well get this part landed. r? @emilio <!-- Reviewable:start --> --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/14662) <!-- Reviewable:end -->
|
|
bholley
changed the title
|
I have some more commits which I'll roll into this PR given that it hasn't landed yet. |
|
CCing a few people who might be happy to hear that the parallel traversal is now fully safe. @pcwalton @SimonSapin @metajack @larsbergstrom @dherman @heycam @upsuper @jdm @nox @mbrubeck @nikomatsakis @Manishearth |
|
This is awesome news! |
|
Awesome! r=me with that line removed, and probably a size check at least for |
| @@ -143,7 +140,7 @@ fn construct_flows_at<'a, N>(context: &LayoutContext<'a>, | |||
| el.mutate_data().unwrap().persist(); | |||
| unsafe { el.unset_dirty_descendants(); } | |||
|
|
|||
| remove_from_bloom_filter(&context.shared.style_context, root, el); | |||
| thread_local.style_context.bloom_filter.maybe_pop(el); | |||
This comment has been minimized.
This comment has been minimized.
emilio
Dec 22, 2016
•
Member
Just drop this line, I'm not really sure it's worth to keep it. The only reason it (IIRC) was there was to help a bit the sequential traversal, and (maybe) eat the bloom filter when the traversal ended (but that was only done in one of the workers, so oh well), or the generation didn't match (but now there's no generation, which is awesome).
| /// objects to other threads. | ||
|
|
||
| #[derive(Clone, Debug, PartialEq)] | ||
| pub struct SendNode<N: TNode>(N); |
This comment has been minimized.
This comment has been minimized.
emilio
Dec 22, 2016
Member
Please add a size_of test in the unit tests with Servo's DOM (and presumably with Gecko's), to ensure this type stays small, since we allocate it all over the place.
|
Oh, also, either here or in a followup, remove all the restyle generation code, which I think is unused now. |
|
@bors-servo r=emilio |
|
|
Hoist bloom filter into scoped TLS, and remove a bunch of complexity and unsafety from the style system With this PR, the only remaining usage of UnsafeNode is the transition stuff, which is servo-only and probably going to be rewritten over the course of stylo. The parallel traversal is now fully typechecked and safe. \o/ r? @emilio <!-- Reviewable:start --> --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/14662) <!-- Reviewable:end -->
|
|
bholley commentedDec 21, 2016
•
edited
With this PR, the only remaining usage of UnsafeNode is the transition stuff, which is servo-only and probably going to be rewritten over the course of stylo. The parallel traversal is now fully typechecked and safe. \o/
r? @emilio
This change is