Bug 185329 - Make a compact version of VariableEnvironment that UnlinkedFunctionExecutable stores and hash-cons these compact environments as we make them
Summary: Make a compact version of VariableEnvironment that UnlinkedFunctionExecutable...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Saam Barati
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2018-05-04 15:43 PDT by Saam Barati
Modified: 2018-05-07 21:18 PDT (History)
15 users (show)

See Also:


Attachments
WIP (13.54 KB, patch)
2018-05-05 11:29 PDT, Saam Barati
no flags Details | Formatted Diff | Diff
patch (17.05 KB, patch)
2018-05-07 16:35 PDT, Saam Barati
msaboff: review+
Details | Formatted Diff | Diff
patch for landing (17.20 KB, patch)
2018-05-07 19:24 PDT, Saam Barati
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Saam Barati 2018-05-04 15:43:17 PDT
....
Comment 1 Saam Barati 2018-05-05 11:29:03 PDT
Created attachment 339653 [details]
WIP

seems to work, just need to clean up a few things.
Comment 2 EWS Watchlist 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.
Comment 3 Saam Barati 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
Comment 4 Saam Barati 2018-05-07 16:35:22 PDT
Created attachment 339765 [details]
patch
Comment 5 EWS Watchlist 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.
Comment 6 Saam Barati 2018-05-07 16:41:56 PDT
<rdar://problem/39961536>
Comment 7 Michael Saboff 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.
Comment 8 Mark Lam 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/?
Comment 9 Saam Barati 2018-05-07 19:24:21 PDT
Created attachment 339788 [details]
patch for landing
Comment 10 WebKit Commit Bot 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>
Comment 11 WebKit Commit Bot 2018-05-07 21:18:32 PDT
All reviewed patches have been landed.  Closing bug.