WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
98541
Switch CachedResource over from SharedBuffer to a new ResourceBuffer
https://bugs.webkit.org/show_bug.cgi?id=98541
Summary
Switch CachedResource over from SharedBuffer to a new ResourceBuffer
Brady Eidson
Reported
2012-10-05 11:25:00 PDT
Switch CachedResource over from SharedBuffer to a new ResourceBuffer The ResourceBuffer class is a shared data buffer - much like SharedBuffer - explicitly meant for the resource cache. From the start it will be a simple wrapper around SharedBuffer and will represent an in-process data buffer. But its purpose is to be the foundation for a buffer than can optionally be hosted differently from a SharedBuffer - Such as shared memory.
Attachments
Patch v1 - Check EWS
(41.46 KB, patch)
2012-10-08 14:21 PDT
,
Brady Eidson
no flags
Details
Formatted Diff
Diff
Patch v2 - Updated and should apply/build.
(41.45 KB, patch)
2012-10-09 13:59 PDT
,
Brady Eidson
buildbot
: commit-queue-
Details
Formatted Diff
Diff
Patch v3 - Try to fix WebKit/win build error
(42.87 KB, patch)
2012-10-09 16:26 PDT
,
Brady Eidson
buildbot
: commit-queue-
Details
Formatted Diff
Diff
Patch v4 - More windows build fixing
(42.88 KB, patch)
2012-10-09 18:03 PDT
,
Brady Eidson
buildbot
: commit-queue-
Details
Formatted Diff
Diff
Patch v5
(42.89 KB, patch)
2012-10-10 10:09 PDT
,
Brady Eidson
andersca
: review+
Details
Formatted Diff
Diff
Show Obsolete
(4)
View All
Add attachment
proposed patch, testcase, etc.
Adam Barth
Comment 1
2012-10-05 11:34:27 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.
Brady Eidson
Comment 2
2012-10-05 11:39:58 PDT
(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.)
Adam Barth
Comment 3
2012-10-05 11:50:21 PDT
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. :)
Brady Eidson
Comment 4
2012-10-05 15:00:50 PDT
(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.
Adam Barth
Comment 5
2012-10-05 15:05:32 PDT
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.
Brady Eidson
Comment 6
2012-10-08 14:21:36 PDT
Created
attachment 167606
[details]
Patch v1 - Check EWS
Brady Eidson
Comment 7
2012-10-09 11:59:12 PDT
Since we've reached agreement on webkit-dev on the nature of this effort... Moving forward with the ResourceBuffer refactoring. r?
Brady Eidson
Comment 8
2012-10-09 12:00:17 PDT
And I'll start with a new patch since this one apparently isn't applying.
Brady Eidson
Comment 9
2012-10-09 13:59:17 PDT
Created
attachment 167843
[details]
Patch v2 - Updated and should apply/build.
Build Bot
Comment 10
2012-10-09 15:24:53 PDT
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
Brady Eidson
Comment 11
2012-10-09 16:26:48 PDT
Created
attachment 167873
[details]
Patch v3 - Try to fix WebKit/win build error
Build Bot
Comment 12
2012-10-09 17:55:08 PDT
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
Brady Eidson
Comment 13
2012-10-09 18:03:19 PDT
Created
attachment 167887
[details]
Patch v4 - More windows build fixing
Build Bot
Comment 14
2012-10-09 22:49:41 PDT
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
Brady Eidson
Comment 15
2012-10-10 09:18:12 PDT
(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.
Brady Eidson
Comment 16
2012-10-10 09:44:06 PDT
(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.
Brady Eidson
Comment 17
2012-10-10 10:09:24 PDT
Created
attachment 168024
[details]
Patch v5 I could reproduce. Fixed, and this patch should be final.
Brady Eidson
Comment 18
2012-10-10 13:23:26 PDT
http://trac.webkit.org/changeset/130947
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug