RESOLVED FIXED 198407
[JSC] InferredValue should not be a JSCell
https://bugs.webkit.org/show_bug.cgi?id=198407
Summary [JSC] InferredValue should not be a JSCell
Yusuke Suzuki
Reported 2019-05-30 20:24:19 PDT
All FunctionExecutable has this. And we do not have interesting policy like CodeBlock edge. We should embed this feature directly to FunctionExecutable so that we can save memory consumed by InferredValue.
Attachments
Patch (66.90 KB, patch)
2019-05-31 23:56 PDT, Yusuke Suzuki
no flags
Archive of layout-test-results from ews211 for win-future (13.55 MB, application/zip)
2019-06-01 04:35 PDT, EWS Watchlist
no flags
Patch (66.94 KB, patch)
2019-06-01 16:14 PDT, Yusuke Suzuki
no flags
Patch (68.27 KB, patch)
2019-06-03 23:03 PDT, Yusuke Suzuki
no flags
Yusuke Suzuki
Comment 1 2019-05-31 21:35:12 PDT
We can create very hacky & fany InferredValue<> thing, and then, we can remove InferredValue completely, and gets 2-3 MB memory reduction in Gmail.
Radar WebKit Bug Importer
Comment 2 2019-05-31 23:04:44 PDT
Yusuke Suzuki
Comment 3 2019-05-31 23:56:14 PDT
EWS Watchlist
Comment 4 2019-06-01 04:35:22 PDT
Comment on attachment 371109 [details] Patch Attachment 371109 [details] did not pass win-ews (win): Output: https://webkit-queues.webkit.org/results/12347214 New failing tests: storage/indexeddb/modern/get-keyrange.html
EWS Watchlist
Comment 5 2019-06-01 04:35:25 PDT
Created attachment 371112 [details] Archive of layout-test-results from ews211 for win-future The attached test failures were seen while running run-webkit-tests on the win-ews. Bot: ews211 Port: win-future Platform: CYGWIN_NT-10.0-17763-3.0.5-338.x86_64-x86_64-64bit
Yusuke Suzuki
Comment 6 2019-06-01 16:14:35 PDT
Yusuke Suzuki
Comment 7 2019-06-03 23:03:32 PDT
Yusuke Suzuki
Comment 8 2019-06-04 10:56:47 PDT
Comment on attachment 371250 [details] Patch Thanks, landing.
WebKit Commit Bot
Comment 9 2019-06-04 11:28:06 PDT
Comment on attachment 371250 [details] Patch Clearing flags on attachment: 371250 Committed r246073: <https://trac.webkit.org/changeset/246073>
WebKit Commit Bot
Comment 10 2019-06-04 11:28:08 PDT
All reviewed patches have been landed. Closing bug.
Saam Barati
Comment 11 2019-06-04 18:46:16 PDT
Comment on attachment 371250 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=371250&action=review > Source/JavaScriptCore/ChangeLog:16 > + InferredValue<> needs validation in GC finalize phase. So this patch also makes SymbolTable Iso-allocated. I don’t understand why that’s needed? > Source/JavaScriptCore/runtime/InferredValue.h:257 > + m_data = (bitwise_cast<uintptr_t>(value) & ValueMask) | encodeState(IsWatched); Nit: Worth asserting the bits flowing in stay the same when masked
Yusuke Suzuki
Comment 12 2019-06-04 19:13:39 PDT
Comment on attachment 371250 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=371250&action=review >> Source/JavaScriptCore/ChangeLog:16 >> + InferredValue<> needs validation in GC finalize phase. So this patch also makes SymbolTable Iso-allocated. > > I don’t understand why that’s needed? Old InferredValue does not mark the child cell. And every time GC finalizes, InferredValue checks the liveness of the child, and if it is live, keeps holding it, otherwise, invalidates itself and clear the child. So this is something like a Weak<> + Watchpoint. We want to keep this semantics in new InferredValue<>. So what we are doing is, we don't visit InferredValue's value (as the same to old InferredValue), and then each finalizer of the owner classes validates the contained cell, as InferedValue did before. Then, we want to execute unconditional finalizer of the owner cell of InferredValue<>, which is FunctionExecutable and SymbolTable. Previously, this was InferredValue cell. >> Source/JavaScriptCore/runtime/InferredValue.h:257 >> + m_data = (bitwise_cast<uintptr_t>(value) & ValueMask) | encodeState(IsWatched); > > Nit: Worth asserting the bits flowing in stay the same when masked Sounds nice, I'll do it in a follow-up patch.
Note You need to log in before you can comment on or make changes to this bug.