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

Make image cache per-document rather than global #15701

Closed
jdm opened this issue Feb 23, 2017 · 6 comments
Closed

Make image cache per-document rather than global #15701

jdm opened this issue Feb 23, 2017 · 6 comments

Comments

@jdm
Copy link
Member

@jdm jdm commented Feb 23, 2017

This would remove any issues related to multiple pages loading the same URL but using different request parameters, and would also avoid the need for synchronous IPC every time the image cache needs to be queried. Rather than a separate thread with an event loop, we'll want a threadsafe data structure that can be used from both the script and layout threads.

@jdm
Copy link
Member Author

@jdm jdm commented Feb 27, 2017

This would help #14993.

@jdm
Copy link
Member Author

@jdm jdm commented Feb 28, 2017

@ferjm How does this sound as larger project to work on? It's a step on the way to implementing the HTML specification's model for loading images (#11517).

@ferjm
Copy link
Member

@ferjm ferjm commented Mar 1, 2017

Sounds very interesting. Happy to work on it. Thank you @jdm

@ferjm ferjm self-assigned this Mar 1, 2017
@ferjm ferjm added the C-assigned label Mar 1, 2017
@metajack
Copy link
Contributor

@metajack metajack commented Mar 14, 2017

This comes at the cost of memory though. Is the image cache slow, or is this motivated solely by messaging races?

@jdm
Copy link
Member Author

@jdm jdm commented Mar 15, 2017

Gecko is working on removing as much synchronous IPC as possible. If we do a per-similar-origin cache, we should get the main memory benefits of sharing that a global cache provides today.

@metajack
Copy link
Contributor

@metajack metajack commented Mar 15, 2017

I guess one point is that if we're going to make it a threadsafe data structure, why restrict it to similar-origin? Why not just pass the reference to the threadsafe hashmap (or whatever) in place of the channel we currently use? This gives us one cache per content process.

bors-servo added a commit that referenced this issue Mar 21, 2017
Make image cache per-document rather than global

- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [X] These changes fix #15701.
- [X] These changes do not require new tests because there should already be WPTs for image loads.

<!-- 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/16048)
<!-- Reviewable:end -->
bors-servo added a commit that referenced this issue Mar 27, 2017
Make image cache per-document rather than global

- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [X] These changes fix #15701.
- [X] These changes do not require new tests because there should already be WPTs for image loads.

<!-- 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/16048)
<!-- Reviewable:end -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

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