| Summary: | Bytecode cache should not encode the SourceProvider for UnlinkedFunctionExecutable's classSource | ||||||
|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Tadeu Zagallo <tzagallo> | ||||
| Component: | JavaScriptCore | Assignee: | Tadeu Zagallo <tzagallo> | ||||
| Status: | RESOLVED FIXED | ||||||
| Severity: | Normal | CC: | commit-queue, ews-watchlist, fpizlo, keith_miller, mark.lam, msaboff, saam, webkit-bug-importer | ||||
| Priority: | P2 | Keywords: | InRadar | ||||
| Version: | WebKit Nightly Build | ||||||
| Hardware: | Unspecified | ||||||
| OS: | Unspecified | ||||||
| See Also: | https://bugs.webkit.org/show_bug.cgi?id=196938 | ||||||
| Bug Depends on: | |||||||
| Bug Blocks: | 196881 | ||||||
| Attachments: |
|
||||||
|
Description
Tadeu Zagallo
2019-04-12 14:21:49 PDT
Created attachment 367346 [details]
Patch
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? 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. 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? (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. 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. Comment on attachment 367346 [details] Patch Clearing flags on attachment: 367346 Committed r244300: <https://trac.webkit.org/changeset/244300> All reviewed patches have been landed. Closing bug. |