| Summary: | Switch CachedResource over from SharedBuffer to a new ResourceBuffer | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Brady Eidson <beidson> | ||||||||||||
| Component: | Page Loading | Assignee: | Brady Eidson <beidson> | ||||||||||||
| Status: | RESOLVED FIXED | ||||||||||||||
| Severity: | Normal | CC: | abarth, andersca, ap, donggwan.kim, gyuyoung.kim, japhet, koivisto, rakuco, sam, webkit.review.bot | ||||||||||||
| Priority: | P2 | ||||||||||||||
| Version: | 528+ (Nightly build) | ||||||||||||||
| Hardware: | All | ||||||||||||||
| OS: | All | ||||||||||||||
| Bug Depends on: | |||||||||||||||
| Bug Blocks: | 98539 | ||||||||||||||
| Attachments: |
|
||||||||||||||
|
Description
Brady Eidson
2012-10-05 11:25:00 PDT
Perhaps we should discuss these changes on webkit-dev first? I'd be interested in know what changes you're planning on making to WebCore. (In reply to comment #1) > Perhaps we should discuss these changes on webkit-dev first? I'd be interested in know what changes you're planning on making to WebCore. The patch I'm preparing for this particular bug is straight forward. It adds a new layer on top of SharedBuffer with zero change in behavior for ports that don't want it (which I assume is all ports besides WK2 ports that opt in to the feature.) I guess I'm interested in the bigger picture of what changes you'll be making to WebCore in this area. In particular, I'd like a chance to convince you that a shared memory cache is overengineering. :) (In reply to comment #3) > I guess I'm interested in the bigger picture of what changes you'll be making to WebCore in this area. In particular, I'd like a chance to convince you that a shared memory cache is overengineering. :) Some refactoring to WebCore boils down to almost direct replacement-style refactoring and should bother few folks, if any. Once such patch should be coming to this bug soon and I have a few others in the pipeline right behind it. There's almost certainly going to have to be some sort of structural changes to the CachedResource/ResourceLoader family of classes. I think everyone can agree that the WebCore/loader directory is a mess of code and needs some love anyways. So it follows we intend for such refactoring to be an opportunity to clean the code, not just shoehorn in what we need. As far as convincing us a shared memory cache is over-engineering... we simply have data that shows it's a win with some basic process models. If you have data that shows it is *NOT* a win, that'd be super interesting. I would prefer to discuss this topic on webkit-dev since the scope is larger than this one bug. In particular, I'd be interested in seeing the data you refer to. Created attachment 167606 [details]
Patch v1 - Check EWS
Since we've reached agreement on webkit-dev on the nature of this effort... Moving forward with the ResourceBuffer refactoring. r? And I'll start with a new patch since this one apparently isn't applying. Created attachment 167843 [details]
Patch v2 - Updated and should apply/build.
Comment on attachment 167843 [details] Patch v2 - Updated and should apply/build. Attachment 167843 [details] did not pass win-ews (win): Output: http://queues.webkit.org/results/14245240 Created attachment 167873 [details]
Patch v3 - Try to fix WebKit/win build error
Comment on attachment 167873 [details] Patch v3 - Try to fix WebKit/win build error Attachment 167873 [details] did not pass win-ews (win): Output: http://queues.webkit.org/results/14244397 Created attachment 167887 [details]
Patch v4 - More windows build fixing
Comment on attachment 167887 [details] Patch v4 - More windows build fixing Attachment 167887 [details] did not pass mac-ews (mac): Output: http://queues.webkit.org/results/14256009 New failing tests: http/tests/misc/location-origin.html (In reply to comment #14) > (From update of attachment 167887 [details]) > Attachment 167887 [details] did not pass mac-ews (mac): > Output: http://queues.webkit.org/results/14256009 > > New failing tests: > http/tests/misc/location-origin.html Seems like a real failure I can reproduce with *only* this patch rolled in, as opposed to the 2 others I have built on top of it. Will be an easy fix, working on it now. (In reply to comment #15) > (In reply to comment #14) > > (From update of attachment 167887 [details] [details]) > > Attachment 167887 [details] [details] did not pass mac-ews (mac): > > Output: http://queues.webkit.org/results/14256009 > > > > New failing tests: > > http/tests/misc/location-origin.html > > Seems like a real failure I can reproduce with *only* this patch rolled in, as opposed to the 2 others I have built on top of it. Will be an easy fix, working on it now. Nevermind, can't reproduce... Digging into the crash log on the bots. Created attachment 168024 [details]
Patch v5
I could reproduce. Fixed, and this patch should be final.
|