[webkit-reviews] review granted: [Bug 194675] Move bytecode cache-related filesystem code out of CodeCache : [Attachment 362322] Patch

[bugzilla-daemon at webkit.org]

Saam Barati <sbarati at apple.com> has granted Tadeu Zagallo
<tzagallo at apple.com>'s request for review:
Bug 194675: Move bytecode cache-related filesystem code out of CodeCache
https://bugs.webkit.org/show_bug.cgi?id=194675

Attachment 362322: Patch

https://bugs.webkit.org/attachment.cgi?id=362322&action=review




--- Comment #20 from Saam Barati <sbarati at apple.com> ---
Comment on attachment 362322
  --> https://bugs.webkit.org/attachment.cgi?id=362322
Patch

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

r=me

> Source/JavaScriptCore/ChangeLog:9
> +	   That code is only used for the bytecode-cache tests, so it should
live in
> +	   jsc.cpp rather than in the CodeCache.

You should make a couple comments about how this changes the interface for
caching.

> Source/JavaScriptCore/parser/SourceProvider.h:120
> +	   virtual void cacheBytecode(const BytecodeCacheGenerator&) const { }

I agree this is nicer than the canCache and solves the issue you found.

> Source/JavaScriptCore/runtime/CodeCache.cpp:202
> +    key.source().provider().cacheBytecode([&]() {

style nit: no need for "()"