WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
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
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.
Top of Page
Format For Printing
XML
Clone This Bug