-
Notifications
You must be signed in to change notification settings - Fork 282
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
Introduce SerializedDisplayItem to reduce mem copies on DL construction #3361
Conversation
With @jrmuizel help to set up the tool, it looks like the item copies are all gone for good 🎉 .
The new log doesn't have any of those:
|
@jrmuizel r? this is ready, I believe, but you need to double check if this makes sense. The memcopies are gone, and Gecko try is green. I'm not seeing a significant difference in Talos, but maybe I'm looking at the wrong thing :) |
But it seems like a worthwhile change to me, will wait to see what @jrmuizel thinks. |
I think this is fine to take now. It moves the unwrap() back between SpecificDisplayItem construction and serialization which reduces our chances of inlining serialization and constant folding the SpecificDisplayItem into that code. However, because of rust-lang/rust#54878 we're not in a great position to achieve this right now. @bors-servo r+ |
📌 Commit f72bf26 has been approved by |
Introduce SerializedDisplayItem to reduce mem copies on DL construction This PR addresses the part of #3358 about DL construction: instead of constructing the display items and associated data and passing through the serializer by value, we just pass the references around, which guarantees that no extra copies are made. TODO: - [x] verify that the copies are gone (need the tool hosted/opened somewhere) - [x] Gecko try push with talos jobs <!-- Reviewable:start --> --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/webrender/3361) <!-- Reviewable:end -->
☀️ Test successful - status-appveyor, status-taskcluster |
…67e04913e9fb (WR PR #3361). r=kats servo/webrender#3361 Differential Revision: https://phabricator.services.mozilla.com/D13246 --HG-- extra : moz-landing-system : lando
…67e04913e9fb (WR PR #3361). r=kats servo/webrender#3361 Differential Revision: https://phabricator.services.mozilla.com/D13246
…67e04913e9fb (WR PR #3361). r=kats servo/webrender#3361 Differential Revision: https://phabricator.services.mozilla.com/D13246 UltraBlame original commit: 3dcdcddd94ff74e05fb4793e970433772ac99098
…67e04913e9fb (WR PR #3361). r=kats servo/webrender#3361 Differential Revision: https://phabricator.services.mozilla.com/D13246 UltraBlame original commit: 3dcdcddd94ff74e05fb4793e970433772ac99098
…67e04913e9fb (WR PR #3361). r=kats servo/webrender#3361 Differential Revision: https://phabricator.services.mozilla.com/D13246 UltraBlame original commit: 3dcdcddd94ff74e05fb4793e970433772ac99098
This PR addresses the part of #3358 about DL construction: instead of constructing the display items and associated data and passing through the serializer by value, we just pass the references around, which guarantees that no extra copies are made.
TODO:
This change is![Reviewable](https://camo.githubusercontent.com/1541c4039185914e83657d3683ec25920c672c6c5c7ab4240ee7bff601adec0b/68747470733a2f2f72657669657761626c652e696f2f7265766965775f627574746f6e2e737667)