Bug 195992

Summary: [JSC] Use finalizer in JSGlobalLexicalEnvironment and JSGlobalObject
Product: WebKit Reporter: Yusuke Suzuki <ysuzuki>
Component: New BugsAssignee: Yusuke Suzuki <ysuzuki>
Status: RESOLVED FIXED    
Severity: Normal CC: ews-watchlist, keith_miller, mark.lam, rniwa, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch
none
Patch
none
Archive of layout-test-results from ews104 for mac-highsierra-wk2
none
Archive of layout-test-results from ews122 for ios-simulator-wk2
none
Patch for landing none

Description Yusuke Suzuki 2019-03-20 01:18:10 PDT
[JSC] Use finalizer in JSGlobalLexicalEnvironment and JSGlobalObject
Comment 1 Yusuke Suzuki 2019-03-20 01:25:41 PDT
Created attachment 365326 [details]
Patch
Comment 2 Yusuke Suzuki 2019-03-20 01:27:24 PDT
Comment on attachment 365326 [details]
Patch

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

> Source/JavaScriptCore/ChangeLog:12
> +        memory / performance problems (actually, previously we used finalizer for ArrayPrototype due to the same reason, and it does not show any problems).

BTW, I'm now planning to reorganize JSScope things to make JSScope cells non JSObject. Once it is done, we can remove this finalizer too (use destructible JSCell for JSGlobalLexcailEnvrionment, and JSDestrucitbleObject for JSGlobalObject).
Comment 3 EWS Watchlist 2019-03-20 05:23:16 PDT
Comment on attachment 365326 [details]
Patch

Attachment 365326 [details] did not pass jsc-ews (mac):
Output: https://webkit-queues.webkit.org/results/11578437

New failing tests:
stress/regress-189028.js.no-cjit-validate-phases
stress/regress-189028.js.bytecode-cache
stress/regress-189028.js.ftl-eager-no-cjit
stress/regress-189028.js.dfg-eager-no-cjit-validate
stress/regress-189028.js.dfg-eager
stress/regress-189028.js.ftl-no-cjit-no-inline-validate
stress/regress-189028.js.no-llint
stress/regress-189028.js.default
stress/regress-189028.js.no-ftl
stress/regress-189028.js.dfg-maximal-flush-validate-no-cjit
stress/regress-189028.js.ftl-eager
stress/regress-189028.js.ftl-no-cjit-small-pool
stress/regress-189028.js.ftl-no-cjit-no-put-stack-validate
stress/regress-189028.js.ftl-eager-no-cjit-b3o1
stress/regress-189028.js.ftl-no-cjit-b3o0
stress/regress-189028.js.ftl-no-cjit-validate-sampling-profiler
stress/regress-189028.js.no-cjit-collect-continuously
apiTests
Comment 4 Yusuke Suzuki 2019-03-20 11:12:07 PDT
Created attachment 365363 [details]
Patch
Comment 5 Keith Miller 2019-03-20 11:20:40 PDT
Comment on attachment 365363 [details]
Patch

r=me.
Comment 6 Mark Lam 2019-03-20 11:22:52 PDT
Comment on attachment 365363 [details]
Patch

r=me too
Comment 7 Yusuke Suzuki 2019-03-20 11:42:26 PDT
Created attachment 365369 [details]
Patch

Fix WebCore side. Use cellHeapCellType since JSSegmentedVariableObject already set finalizer.
Comment 8 Yusuke Suzuki 2019-03-20 11:43:55 PDT
Created attachment 365371 [details]
Patch

Fix WebCore side. Use cellHeapCellType since JSSegmentedVariableObject already set finalizer.
Comment 9 Yusuke Suzuki 2019-03-20 12:17:38 PDT
Created attachment 365382 [details]
Patch
Comment 10 EWS Watchlist 2019-03-20 16:51:04 PDT
Comment on attachment 365382 [details]
Patch

Attachment 365382 [details] did not pass mac-wk2-ews (mac-wk2):
Output: https://webkit-queues.webkit.org/results/11586909

New failing tests:
http/wpt/mediarecorder/MediaRecorder-AV-audio-video-dataavailable.html
Comment 11 EWS Watchlist 2019-03-20 16:51:06 PDT
Created attachment 365438 [details]
Archive of layout-test-results from ews104 for mac-highsierra-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews104  Port: mac-highsierra-wk2  Platform: Mac OS X 10.13.6
Comment 12 EWS Watchlist 2019-03-20 17:21:01 PDT
Comment on attachment 365382 [details]
Patch

Attachment 365382 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: https://webkit-queues.webkit.org/results/11586830

New failing tests:
fast/visual-viewport/ios/min-scale-greater-than-one.html
Comment 13 EWS Watchlist 2019-03-20 17:21:02 PDT
Created attachment 365444 [details]
Archive of layout-test-results from ews122 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews122  Port: ios-simulator-wk2  Platform: Mac OS X 10.13.6
Comment 14 Yusuke Suzuki 2019-03-20 21:20:11 PDT
Created attachment 365480 [details]
Patch for landing
Comment 15 Yusuke Suzuki 2019-03-20 21:20:38 PDT
Comment on attachment 365480 [details]
Patch for landing

Oops, accidentally cleared r+.
Comment 16 Yusuke Suzuki 2019-03-20 21:21:10 PDT
(In reply to Build Bot from comment #13)
> Created attachment 365444 [details]
> Archive of layout-test-results from ews122 for ios-simulator-wk2
> 
> The attached test failures were seen while running run-webkit-tests on the
> ios-sim-ews.
> Bot: ews122  Port: ios-simulator-wk2  Platform: Mac OS X 10.13.6

After checking these crashes, it seems unrelated. I'll land it after rechecking EWS status.
Comment 17 Yusuke Suzuki 2019-03-21 00:00:28 PDT
Committed r243279: <https://trac.webkit.org/changeset/243279>
Comment 18 Radar WebKit Bug Importer 2019-03-21 00:01:35 PDT
<rdar://problem/49097562>