Bug 198482 – JSScript should not keep bytecode cache in memory

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Tadeu Zagallo]

...

[Tadeu Zagallo]

Created attachment 371185 [details]
Patch

[Tadeu Zagallo]

Created attachment 371189 [details]
Patch

Fix windows build

[Saam Barati]

Comment on attachment 371189 [details]
Patch

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

r=me

> Source/JavaScriptCore/runtime/CachedTypes.cpp:176
> +    Ref<CachedBytecode> releaseMapped(BytecodeCacheError& error)

Kinda weird this returns ref instead of ptr. Why not RefPtr and make nullptr mean there was an error? I feel like it's weird to construct an empty CachedBytecode here.

> Source/JavaScriptCore/runtime/CachedTypes.cpp:180
> +            error.standardError(errno);

nit: This style feels kinda weird. Why not just make error something you assign to? And by default it's empty? And you can make this a static function that is a factory constructor. I feel like that's more in line with the style of error handling I've seen elsewhere in WK. So something like:
```
error = BytecodeCacheError::standardError(errno)
return nullptr 
```

> Source/JavaScriptCore/runtime/CachedTypes.cpp:2411
> +    Encoder encoder(vm, -1);

You could also make -1 the default argument of this function.

[Tadeu Zagallo]

Created attachment 371264 [details]
Patch for landing

[WebKit Commit Bot]

Comment on attachment 371264 [details]
Patch for landing

Clearing flags on attachment: 371264

Committed r246060: <https://trac.webkit.org/changeset/246060>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/51392697>