Bug 134560 – Buffer CSS and JS resources in network process before sending over to web process

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Pratik Solanki]

Buffer CSS and JS resources in network process before sending over to web process

[Pratik Solanki]

Created attachment 234280 [details]
Patch

[Pratik Solanki]

Initial implementation with USE(CFNETWORK) support for iOS. I'm looking into doing this for mac as well.

[Brady Eidson]

Is there actually no advantage to streaming JS and CSS resources as the bytes come in?  It seems like the JIT could get started on an incomplete resource, saving time later...?

[Pratik Solanki]

(In reply to comment #3)
> Is there actually no advantage to streaming JS and CSS resources as the bytes come in?

I don't know if it's possible to start parsing,JITting JS or CSS as the bytes come in. It's certainly not something we do right now.

>  It seems like the JIT could get started on an incomplete resource, saving time later...?

I guess if we start doing that and it proves to be a perf win, then we can disable this optimization.

[Pratik Solanki]

Created attachment 234299 [details]
Patch

[Pratik Solanki]

<rdar://problem/17541403>

[Alexey Proskuryakov]

Comment on attachment 234299 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=234299&action=review

Looks like a good idea, assuming that it doesn't break progress events or resource load timing or Web Inspector (which are all too involved for me to assess at the moment, but at least tests pass).

> Source/WebKit2/NetworkProcess/NetworkResourceLoader.h:199
> +    RefPtr<WebCore::SharedBuffer> m_bufferedData;

It seems super unfortunate that sync loads collect the data in SynchronousNetworkLoaderClient object, but all loads have a data member in NetworkResourceLoader.

I think that this should be designed in a uniform way, definitely without having multiple instances of the data member.

[Antti Koivisto]

I already had a patch for this in https://bugs.webkit.org/show_bug.cgi?id=132229 also covering image resources.

[Alexey Proskuryakov]

Correct. The problem with that patch was that it coalesced way too much, needlessly and dangerously changing semantics of network loader.

[Pratik Solanki]

Yeah, this one is a bit more targeted and simpler. But the idea is the same.

[Pratik Solanki]

(In reply to comment #7)
> (From update of attachment 234299 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=234299&action=review
> 
> Looks like a good idea, assuming that it doesn't break progress events or resource load timing or Web Inspector (which are all too involved for me to assess at the moment, but at least tests pass).
> 
> > Source/WebKit2/NetworkProcess/NetworkResourceLoader.h:199
> > +    RefPtr<WebCore::SharedBuffer> m_bufferedData;
> 
> It seems super unfortunate that sync loads collect the data in SynchronousNetworkLoaderClient object, but all loads have a data member in NetworkResourceLoader.
> 
> I think that this should be designed in a uniform way, definitely without having multiple instances of the data member.

Good idea. I can definitely merge them and make the code more efficient for SynchronousLoaderClient.

[Pratik Solanki]

Created attachment 234574 [details]
Patch

This depends on patches from bug 134731, bug 134732 and bug 134733.

https://bugs.webkit.org/attachment.cgi?id=234571&action=review
https://bugs.webkit.org/attachment.cgi?id=234572&action=review
https://bugs.webkit.org/attachment.cgi?id=234573&action=review

[Pratik Solanki]

Created attachment 234673 [details]
Patch for bots that will apply cleanly now

[Pratik Solanki]

Committed r170958: <http://trac.webkit.org/changeset/170958>

[Antti Koivisto]

*** Bug 132229 has been marked as a duplicate of this bug. ***