Bug 193646

Summary:

[JSC] Lazily initialize JSModuleLoader

Product:

WebKit

Reporter:

Yusuke Suzuki <ysuzuki>

Component:

New Bugs

Assignee:

Yusuke Suzuki <ysuzuki>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

ews-watchlist, keith_miller, mark.lam, msaboff, saam, webkit-bug-importer

Priority:

Keywords:

InRadar

Version:

WebKit Nightly Build

Hardware:

Unspecified

OS:

Unspecified

Bug Depends on:

Bug Blocks:

193606

Attachments:

Description	Flags
Patch	none
Patch	none
Patch	none

Yusuke Suzuki

Reported 2019-01-20 23:17:04 PST

[JSC] Lazily initialize JSModuleLoader

Attachments
Patch (4.27 KB, patch) 2019-01-20 23:18 PST, Yusuke Suzuki	no flags	Details Formatted Diff Diff
Patch (10.61 KB, patch) 2019-01-20 23:38 PST, Yusuke Suzuki	no flags	Details Formatted Diff Diff
Patch (4.38 KB, patch) 2019-01-21 13:54 PST, Yusuke Suzuki	no flags	Details Formatted Diff Diff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.

Yusuke Suzuki

Comment 1 2019-01-20 23:18:43 PST

Created attachment 359672 [details] Patch

Yusuke Suzuki

Comment 2 2019-01-20 23:38:35 PST

Created attachment 359673 [details] Patch

Keith Miller

Comment 3 2019-01-21 08:45:22 PST

Comment on attachment 359673 [details] Patch r=me.

Saam Barati

Comment 4 2019-01-21 10:42:54 PST

Comment on attachment 359673 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=359673&action=review > Source/JavaScriptCore/runtime/HashMapImpl.h:206 > + static HashMapBuffer* create(ExecState* nullOrExecForOOM, VM& vm, JSCell*, uint32_t capacity) not a fan of this, why not globalExec? > Source/JavaScriptCore/runtime/JSModuleLoader.cpp:109 > + JSMap* map = JSMap::create(nullptr, vm, globalObject->mapStructure()); why not globalExec?

Saam Barati

Comment 5 2019-01-21 10:44:00 PST

Comment on attachment 359673 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=359673&action=review >> Source/JavaScriptCore/runtime/JSModuleLoader.cpp:109 >> + JSMap* map = JSMap::create(nullptr, vm, globalObject->mapStructure()); > > why not globalExec? If you don't want an exception to be thrown, I'd use globalExec, and then releaseAssertNoException here instead of passing in nullptr. I think the abstraction is backwards this way, but not in the way I propose.

Yusuke Suzuki

Comment 6 2019-01-21 13:53:23 PST

Comment on attachment 359673 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=359673&action=review Thanks! >> Source/JavaScriptCore/runtime/HashMapImpl.h:206 >> + static HashMapBuffer* create(ExecState* nullOrExecForOOM, VM& vm, JSCell*, uint32_t capacity) > > not a fan of this, why not globalExec? Personally, rather, I would like to drop this ExecState* parameter for simple `JSMap::create()` / `JSModuleLoader::create` cases because it allocates fixed amount of memory which is not so large. But in the meantime, we still have this parameter. So, using globalExec makes sense. Fixed. >>> Source/JavaScriptCore/runtime/JSModuleLoader.cpp:109 >>> + JSMap* map = JSMap::create(nullptr, vm, globalObject->mapStructure()); >> >> why not globalExec? > > If you don't want an exception to be thrown, I'd use globalExec, and then releaseAssertNoException here instead of passing in nullptr. I think the abstraction is backwards this way, but not in the way I propose. Fixed.

Yusuke Suzuki

Comment 7 2019-01-21 13:54:20 PST

Created attachment 359709 [details] Patch

Yusuke Suzuki

Comment 8 2019-01-21 14:01:22 PST

Committed r240242: <https://trac.webkit.org/changeset/240242>

Radar WebKit Bug Importer

Comment 9 2019-01-21 14:02:28 PST

<rdar://problem/47432806>

Note You need to log in before you can comment on or make changes to this bug.