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

Expire entries in the shared texture cache before allocating another layer #3293

Merged
merged 6 commits into from Nov 9, 2018

Conversation

@bholley
Copy link
Contributor

bholley commented Nov 9, 2018

@bholley
Copy link
Contributor Author

bholley commented Nov 9, 2018

@bors-servo r=gw3583

@bors-servo
Copy link
Contributor

bors-servo commented Nov 9, 2018

📌 Commit df692b5 has been approved by gw3583

@bors-servo
Copy link
Contributor

bors-servo commented Nov 9, 2018

Testing commit df692b5 with merge d2cbffe...

bors-servo added a commit that referenced this pull request Nov 9, 2018
Expire entries in the shared texture cache before allocating another layer

https://bugzilla.mozilla.org/show_bug.cgi?id=1505639

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

bors-servo commented Nov 9, 2018

💔 Test failed - status-appveyor

bholley added 6 commits Nov 8, 2018
This restores non-mac platforms back to the old maximum of 64 layers.

Differential Revision: https://phabricator.services.mozilla.com/D11416
This is a bit cleaner and avoids borrow checking problems.

Differential Revision: https://phabricator.services.mozilla.com/D11271
The container struct reduces repetition, and the helper method will simplify
future work by allowing early returns.

Differential Revision: https://phabricator.services.mozilla.com/D11272
This patch does a few things:
* Uses the same age-based expiration that we use for the standalone
  cache for the shared cache.
* Tracks the last time we expired shared entries.
* When allocation fails, tries to expire old entries before allocating
  another layer, assuming we haven't done so just a few frames ago.
* Eliminates the size limits on the shared caches, and just grows them
  instead of allocating standalone entries.

The last bit could cause us to get stuck with larger total texture
allocations than we do now, if a lot of entries were used in quick
succession (since we never drop shared entries like we do with
standalone entries). However, I think it's probably unlikely enough
that it's fine to ship it for a few days while I finish up shrinking
support.

The parameters here (75 and 25) still aren't perfect, and I'll tune them going
forward.

Differential Revision: https://phabricator.services.mozilla.com/D11273
We need this so that we can have a detail-less enum called EntryKind to
eliminate the boolean. People on #servo agreed this was an appropriate
naming strategy.
@bholley bholley force-pushed the bholley:expire_shared_entries branch from df692b5 to 7d33d12 Nov 9, 2018
@bholley
Copy link
Contributor Author

bholley commented Nov 9, 2018

@bors-servo r=gw3583

@bors-servo
Copy link
Contributor

bors-servo commented Nov 9, 2018

📌 Commit 7d33d12 has been approved by gw3583

@bors-servo
Copy link
Contributor

bors-servo commented Nov 9, 2018

Testing commit 7d33d12 with merge 9e65149...

bors-servo added a commit that referenced this pull request Nov 9, 2018
Expire entries in the shared texture cache before allocating another layer

https://bugzilla.mozilla.org/show_bug.cgi?id=1505639

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

bors-servo commented Nov 9, 2018

☀️ Test successful - status-appveyor, status-taskcluster
Approved by: gw3583
Pushing 9e65149 to master...

@bors-servo bors-servo merged commit 7d33d12 into servo:master Nov 9, 2018
1 of 3 checks passed
1 of 3 checks passed
Taskcluster (pull_request) TaskGroup: failure
Details
continuous-integration/appveyor/pr Waiting for AppVeyor build to complete
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

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