Bug 185329

Summary: Make a compact version of VariableEnvironment that UnlinkedFunctionExecutable stores and hash-cons these compact environments as we make them
Product: WebKit Reporter: Saam Barati <saam>
Component: JavaScriptCoreAssignee: Saam Barati <saam>
Status: RESOLVED FIXED    
Severity: Normal CC: benjamin, commit-queue, ews-watchlist, fpizlo, ggaren, gskachkov, jfbastien, joepeck, keith_miller, mark.lam, msaboff, rmorisset, ticaiolima, webkit-bug-importer, ysuzuki
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
WIP
none
patch
msaboff: review+
patch for landing none

Saam Barati
Reported 2018-05-04 15:43:17 PDT
....
Attachments
WIP (13.54 KB, patch)
2018-05-05 11:29 PDT, Saam Barati 		no flags
DetailsFormatted DiffDiff
patch (17.05 KB, patch)
2018-05-07 16:35 PDT, Saam Barati 		msaboff: review+
DetailsFormatted DiffDiff
patch for landing (17.20 KB, patch)
2018-05-07 19:24 PDT, Saam Barati 		no flags
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Saam Barati
Comment 1 2018-05-05 11:29:03 PDT
Created attachment 339653 [details] WIP seems to work, just need to clean up a few things.
EWS Watchlist
Comment 2 2018-05-05 11:31:13 PDT
Attachment 339653 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/runtime/VM.cpp:145: Alphabetical sorting problem. [build/include_order] [4] Total errors found: 1 in 7 files If any of these errors are false positives, please file a bug against check-webkit-style.
Saam Barati
Comment 3 2018-05-05 12:32:23 PDT
(In reply to Saam Barati from comment #1) > Created attachment 339653 [details] > WIP > > seems to work, just need to clean up a few things. Seems to be a 50-60MB improvement on the workload I am testing
Saam Barati
Comment 4 2018-05-07 16:35:22 PDT
Created attachment 339765 [details] patch
EWS Watchlist
Comment 5 2018-05-07 16:38:33 PDT
Attachment 339765 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/runtime/VM.cpp:145: Alphabetical sorting problem. [build/include_order] [4] Total errors found: 1 in 7 files If any of these errors are false positives, please file a bug against check-webkit-style.
Saam Barati
Comment 6 2018-05-07 16:41:56 PDT
<rdar://problem/39961536>
Michael Saboff
Comment 7 2018-05-07 17:03:27 PDT
Comment on attachment 339765 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=339765&action=review r=me > Source/JavaScriptCore/ChangeLog:9 > + use space to represent and UnlinkedFunctionExecutable's parent TDZ variables. *an* > Source/JavaScriptCore/parser/VariableEnvironment.cpp:101 > + m_isEverythingCaptured = env.isEverythingCaptured(); Change this to a member initializer list.
Mark Lam
Comment 8 2018-05-07 17:25:22 PDT
Comment on attachment 339765 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=339765&action=review > Source/JavaScriptCore/parser/VariableEnvironment.h:205 > + WTF_MAKE_NONCOPYABLE(Handle); // If we wanted to make this copyable, we'd need to do a hashtable lookup and bump the refcount. /refcount/refCount/?
Saam Barati
Comment 9 2018-05-07 19:24:21 PDT
Created attachment 339788 [details] patch for landing
WebKit Commit Bot
Comment 10 2018-05-07 21:18:30 PDT
Comment on attachment 339788 [details] patch for landing Clearing flags on attachment: 339788 Committed r231477: <https://trac.webkit.org/changeset/231477>
WebKit Commit Bot
Comment 11 2018-05-07 21:18:32 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.