Bug 212437

Summary: Limit memory used by wasm/references/multitable.js on memory limited devices
Product: WebKit Reporter: Saam Barati <saam>
Component: JavaScriptCoreAssignee: Saam Barati <saam>
Status: RESOLVED FIXED    
Severity: Normal CC: ews-watchlist, keith_miller, mark.lam, msaboff, ryanhaddad, tzagallo, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
patch
keith_miller: review+
patch for landing
none
patch for landing none

Description Saam Barati 2020-05-27 15:03:03 PDT
...
Comment 1 Saam Barati 2020-05-27 15:03:29 PDT
Currently uses 600MB and jetsams iOS tests
Comment 2 Saam Barati 2020-05-27 16:11:13 PDT
Created attachment 400400 [details]
patch
Comment 3 Keith Miller 2020-05-27 16:13:38 PDT
Comment on attachment 400400 [details]
patch

r=me
Comment 4 Mark Lam 2020-05-27 16:18:46 PDT
Comment on attachment 400400 [details]
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=400400&action=review

> JSTests/wasm/references/multitable.js:428
> +          .End().WebAssembly().get())), Error, "Table count of 1000000 is too big, maximum 1000000");

Parameterize on largeNumber too instead of hardcoding 1000000 in the message?

> Source/JavaScriptCore/jsc.cpp:2497
> +EncodedJSValue JSC_HOST_CALL functionIsMemoryLimited(JSGlobalObject*, CallFrame*)
> +{
> +#if PLATFORM(IOS) || PLATFORM(TVOS) || PLATFORM(WATCHOS)
> +    return JSValue::encode(jsBoolean(true));
> +#else
> +    return JSValue::encode(jsBoolean(false));
> +#endif
> +}

Put this in $vm because it may be useful for LayoutTests too?
Comment 5 Saam Barati 2020-05-27 16:41:33 PDT
Comment on attachment 400400 [details]
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=400400&action=review

>> JSTests/wasm/references/multitable.js:428
>> +          .End().WebAssembly().get())), Error, "Table count of 1000000 is too big, maximum 1000000");
> 
> Parameterize on largeNumber too instead of hardcoding 1000000 in the message?

this one is needed since it's an actual test that 1000000 exceeds the limit

>> Source/JavaScriptCore/jsc.cpp:2497
>> +}
> 
> Put this in $vm because it may be useful for LayoutTests too?

Yeah I can do that
Comment 6 Saam Barati 2020-05-27 17:00:21 PDT
Created attachment 400404 [details]
patch for landing
Comment 7 Saam Barati 2020-05-27 17:04:00 PDT
Created attachment 400405 [details]
patch for landing
Comment 8 EWS 2020-05-27 17:46:33 PDT
Committed r262227: <https://trac.webkit.org/changeset/262227>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 400405 [details].
Comment 9 Radar WebKit Bug Importer 2020-05-27 17:47:19 PDT
<rdar://problem/63698021>