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

Stop reinitializing textures #3183

Merged
merged 7 commits into from Oct 11, 2018
Merged

Conversation

@bholley
Copy link
Contributor

bholley commented Oct 10, 2018

This was reviewed in [1]. @kvark, can you confirm that you don't have any additional feedback? Feel free to trigger landing if you don't.

[1] https://bugzilla.mozilla.org/show_bug.cgi?id=1497665


This change is Reviewable

bholley added 7 commits Oct 4, 2018
This is a prerequisite to switching to immutable storage.

Differential Revision: https://phabricator.services.mozilla.com/D8161
We also drop the vec scheme and just use a hash-map. None of the operations
here are hot enough that the difference between a HashMap and a Vec
should matter, and using a HashMap simplifies things. Glenn says that
hashmap lookups were once hot in profiles, but that was before
FastHashMap and not specifically these lookups.

Differential Revision: https://phabricator.services.mozilla.com/D8164
Things get simpler when reinitialization goes away.

Differential Revision: https://phabricator.services.mozilla.com/D8166
@kvark
Copy link
Member

kvark commented Oct 11, 2018

My concerns have been addressed, and the only remaining one - about hashmap - is being discussed, but it's not a ton of work to bring back the vector indexation in case we need it.
@bors-servo r+

@bors-servo
Copy link
Contributor

bors-servo commented Oct 11, 2018

📌 Commit 788d251 has been approved by kvark

@bors-servo
Copy link
Contributor

bors-servo commented Oct 11, 2018

Testing commit 788d251 with merge 4ea8ed1...

bors-servo added a commit that referenced this pull request Oct 11, 2018
Stop reinitializing textures

This was reviewed in [1]. @kvark, can you confirm that you don't have any additional feedback? Feel free to trigger landing if you don't.

[1] https://bugzilla.mozilla.org/show_bug.cgi?id=1497665

<!-- 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/3183)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Oct 11, 2018

☀️ Test successful - status-appveyor, status-taskcluster
Approved by: kvark
Pushing 4ea8ed1 to master...

@bors-servo bors-servo merged commit 788d251 into servo:master Oct 11, 2018
3 checks passed
3 checks passed
Taskcluster (pull_request) TaskGroup: success
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
homu Test successful
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.