[Webkit-unassigned] [Bug 167415] Avoid evicting link preload resources when parsing is done.

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=167415

--- Comment #21 from Yoav Weiss <yoav at yoav.ws> ---
(In reply to comment #20)
> Comment on attachment 299837 [details]
> Patch
> 
> I guess an alternative would be to restrict m_preloads to speculative
> preloads only.
> Can we do that?

We probably could, but that would probably mean duplicating the existing mechanisms that relate to m_preloads, or generalize them so that they can be applied to two separate lists. Do you think such a separation would be valuable enough to justify that?

> Also, if we do not fully clean m_preloads at onload time, won't we keep
> references to these resources?

We fully clear m_preloads when CachedResourceLoader is terminated, so (IIUC) when the document is detached.

> If so, we might not be able to reclaim memory on these, even if MemoryCache
> is asked to do so.

That's deliberate, as these resources might be used after onload (e.g. by scripts injecting corresponding DOM nodes that need them)

I'm planning to introduce a warning mechanism for unused preloads (probably at 3 seconds after onload). If we want to reclaim memory for these resources before the document detaches, that might be a good point to introduce clearing mechanism for them.

> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=299837&action=review
> 
> > Source/WebCore/ChangeLog:14
> > +        being cleared), said issue is also fixed by clearing previousely preloaded resources if an invalid link preload is later detected.
> 
> What happens if a speculative preload is scheduled first and a link preload
> on the same resource happens after.
> Shouldn't the speculative preload be marked as link preload?

I should probably add handling for that scenario for correctness sake, even if I'm not particularly worried about it.

I think such a scenario would be relatively rare, and even if it happens, clearPreloads(ClearSpeculativePreloads) won't evict this resource from MemoryCache, as it would be referenced by the time clearPreloads is first called.

> 
> > Source/WebCore/loader/cache/CachedResourceLoader.cpp:870
> > +        resource->setLinkPreload();
> 
> This should be done in CachedResource constructor.

OK

> Maybe we should in the future make CachedResource have a
> CachedResourceRequest member.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.webkit.org/pipermail/webkit-unassigned/attachments/20170127/0911c7d0/attachment-0001.html>