Bug 196878 – Bytecode cache should not encode the SourceProvider for UnlinkedFunctionExecutable's classSource

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Tadeu Zagallo]

...

[Tadeu Zagallo]

Created attachment 367346 [details]
Patch

[Saam Barati]

Comment on attachment 367346 [details]
Patch

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

> Source/JavaScriptCore/ChangeLog:14
> +        incremental cache, each UnlinkedFunctionCodeBlock is encoded in isolation, which
> +        means we can no longer deduplicate it and the full program text was being encoded
> +        multiple times in the cache.

How is this just not a general problem that affects all types that were previously being de-duplicated during encoding?

[Filip Pizlo]

Comment on attachment 367346 [details]
Patch

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

>> Source/JavaScriptCore/ChangeLog:14
>> +        multiple times in the cache.
> 
> How is this just not a general problem that affects all types that were previously being de-duplicated during encoding?

I'm also curious.

[Saam Barati]

Comment on attachment 367346 [details]
Patch

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

r=me

>> Source/JavaScriptCore/ChangeLog:14
>> +        multiple times in the cache.
> 
> How is this just not a general problem that affects all types that were previously being de-duplicated during encoding?

can we open a bug so we can handle the general case?

[Saam Barati]

(In reply to Saam Barati from comment #4)
> Comment on attachment 367346 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=367346&action=review
> 
> r=me
> 
> >> Source/JavaScriptCore/ChangeLog:14
> >> +        multiple times in the cache.
> > 
> > How is this just not a general problem that affects all types that were previously being de-duplicated during encoding?
> 
> can we open a bug so we can handle the general case?

Or maybe it's worth just solving instead of solving this one specific case we know is especially bad.

[Tadeu Zagallo]

Comment on attachment 367346 [details]
Patch

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

>>>>> Source/JavaScriptCore/ChangeLog:14
>>>>> +        multiple times in the cache.
>>>> 
>>>> How is this just not a general problem that affects all types that were previously being de-duplicated during encoding?
>>> 
>>> I'm also curious.
>> 
>> can we open a bug so we can handle the general case?
> 
> Or maybe it's worth just solving instead of solving this one specific case we know is especially bad.

This is a general problem, but it's a bit more complicated to solve in the general case. As we discussed on IRC, the encoder map might end up with dead pointers, but maybe we can batch the updates to minimize the duplications. In which case, I still think that this case is bad enough that we *never* want it to be duplicated. Plus, I remember we've discussed in the past that it could be useful to be able to inject a SourceProvider during decoding, instead of allocating a new one. I'll land this as is, so that I can start running JS2 as a bytecode cache test and file another bug fixing the general case.

[WebKit Commit Bot]

Comment on attachment 367346 [details]
Patch

Clearing flags on attachment: 367346

Committed r244300: <https://trac.webkit.org/changeset/244300>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/49920240>