RESOLVED FIXED 193646
[JSC] Lazily initialize JSModuleLoader
https://bugs.webkit.org/show_bug.cgi?id=193646
Summary [JSC] Lazily initialize JSModuleLoader
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
Patch (10.61 KB, patch)
2019-01-20 23:38 PST, Yusuke Suzuki
no flags
Patch (4.38 KB, patch)
2019-01-21 13:54 PST, Yusuke Suzuki
no flags
Yusuke Suzuki
Comment 1 2019-01-20 23:18:43 PST
Yusuke Suzuki
Comment 2 2019-01-20 23:38:35 PST
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
Yusuke Suzuki
Comment 8 2019-01-21 14:01:22 PST
Radar WebKit Bug Importer
Comment 9 2019-01-21 14:02:28 PST
Note You need to log in before you can comment on or make changes to this bug.