Bug 158543 - JSObject::reifyAllStaticProperties cleanup
Summary: JSObject::reifyAllStaticProperties cleanup
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Gavin Barraclough
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-06-08 14:39 PDT by Gavin Barraclough
Modified: 2016-06-09 12:41 PDT (History)
8 users (show)

See Also:


Attachments
Fix (10.75 KB, patch)
2016-06-08 14:50 PDT, Gavin Barraclough
mark.lam: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Gavin Barraclough 2016-06-08 14:39:24 PDT
- JSObject & Structure contain fields labeled 'staticFunctionsReified', however reification now affects all properties, not just functions. Rename to 'staticPropertiesReified'.
- reifyAllStaticProperties relies on a 'hasStaticProperties' method on ClassInfo that walks the ClassInfo inheritance chain looking for static property tables. We can now more efficiently get this information from TypeInfo.
- reifyAllStaticProperties triggers a 'toUncacheableDictionaryTransition'; this is overzealous, cacheable dictionary is sufficient - this is what we do in the case of DOM prototype property reification (see 'reifyStaticProperties' in Lookup.h). (Changing this with an eye on switching DOM prototype property reification to use JSObject:: reifyAllStaticProperties, rather than having its own special purpose code path.)
Comment 1 Gavin Barraclough 2016-06-08 14:50:54 PDT
Created attachment 280842 [details]
Fix
Comment 2 Mark Lam 2016-06-08 14:58:26 PDT
Comment on attachment 280842 [details]
Fix

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

r=me

> Source/JavaScriptCore/ChangeLog:9
> +        affects all properties, not just functions. Rename to 'staticPropertiesReified'.

Missing space?  Or is this a tab?
Comment 3 Gavin Barraclough 2016-06-08 22:42:24 PDT
Committed revision 201853.
Comment 4 Chris Dumez 2016-06-09 09:21:52 PDT
Only 1 data point so it could be noise but it looks like this may be a ~4% regression on Dromaeo DOM Core (especially the DOM Modification and DOM query subtests) on MacBookPro.
Comment 5 Chris Dumez 2016-06-09 12:41:58 PDT
(In reply to comment #4)
> Only 1 data point so it could be noise but it looks like this may be a ~4%
> regression on Dromaeo DOM Core (especially the DOM Modification and DOM
> query subtests) on MacBookPro.

Looks like this was noise, the next data point looks much better.