Bug 215666 - [JSC] Add Object.getOwnPropertyNames caching as it is done for Object.keys, and accelerate Object.getOwnPropertyDescriptor
Summary: [JSC] Add Object.getOwnPropertyNames caching as it is done for Object.keys, a...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Yusuke Suzuki
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2020-08-19 15:14 PDT by Yusuke Suzuki
Modified: 2020-08-19 22:08 PDT (History)
7 users (show)

See Also:

Attachments
Patch (73.79 KB, patch)
2020-08-19 15:18 PDT, Yusuke Suzuki 		no flags Details | Formatted Diff | Diff
Patch (73.89 KB, patch)
2020-08-19 16:01 PDT, Yusuke Suzuki 		no flags Details | Formatted Diff | Diff
Patch (73.89 KB, patch)
2020-08-19 16:03 PDT, Yusuke Suzuki 		saam: review+
Details | Formatted Diff | Diff
Patch for landing (75.37 KB, patch)
2020-08-19 18:56 PDT, Yusuke Suzuki 		no flags Details | Formatted Diff | Diff
Show Obsolete (2) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Yusuke Suzuki 2020-08-19 15:14:22 PDT 
[JSC] Add Object.getOwnPropertyNames caching as it is done for Object.keys, and accelerate Object.getOwnPropertyDescriptor
Comment 1 Yusuke Suzuki 2020-08-19 15:18:32 PDT 
Created attachment 406878 [details]
Patch
Comment 2 Yusuke Suzuki 2020-08-19 16:01:29 PDT 
Created attachment 406881 [details]
Patch
Comment 3 Yusuke Suzuki 2020-08-19 16:03:53 PDT 
Created attachment 406882 [details]
Patch
Comment 4 Saam Barati 2020-08-19 18:00:08 PDT 
Comment on attachment 406882 [details]
Patch

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

Nice. r=me

> Source/JavaScriptCore/dfg/DFGConstantFoldingPhase.cpp:944
> +                            CachedPropertyNames key = node->op() == ObjectGetOwnPropertyNames ? CachedPropertyNames::OwnNames : CachedPropertyNames::OwnKeys;

nit: Just put this inline in the function call below?

> Source/JavaScriptCore/runtime/StructureRareData.cpp:79
> +    auto validateAndAppend = [&](WriteBarrier<JSImmutableButterfly>& slot) {

nit: why not just have this in the loop below?

> Source/JavaScriptCore/runtime/StructureRareData.h:40
> +enum class CachedPropertyNames : uint8_t {

nit: I'd name this "CachedPropertyNamesKind" or similar
Comment 5 Yusuke Suzuki 2020-08-19 18:32:49 PDT 
Comment on attachment 406882 [details]
Patch

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

Thanks!

>> Source/JavaScriptCore/dfg/DFGConstantFoldingPhase.cpp:944
>> +                            CachedPropertyNames key = node->op() == ObjectGetOwnPropertyNames ? CachedPropertyNames::OwnNames : CachedPropertyNames::OwnKeys;
> 
> nit: Just put this inline in the function call below?

Sounds nice, fixed.

>> Source/JavaScriptCore/runtime/StructureRareData.cpp:79
>> +    auto validateAndAppend = [&](WriteBarrier<JSImmutableButterfly>& slot) {
> 
> nit: why not just have this in the loop below?

Ah, right. Originally, they are not loop, But I made it loop by refactoring. We should put it inside loop without lambda. Fixed.

>> Source/JavaScriptCore/runtime/StructureRareData.h:40
>> +enum class CachedPropertyNames : uint8_t {
> 
> nit: I'd name this "CachedPropertyNamesKind" or similar

Fixed.
Comment 6 Yusuke Suzuki 2020-08-19 18:56:23 PDT 
Created attachment 406899 [details]
Patch for landing
Comment 7 Yusuke Suzuki 2020-08-19 22:07:34 PDT 
Committed r265934: <https://trac.webkit.org/changeset/265934>
Comment 8 Radar WebKit Bug Importer 2020-08-19 22:08:14 PDT 
<rdar://problem/67452338>