Bug 158250 - Dictionary property access should be fast
Summary: Dictionary property access should be fast
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: Geoffrey Garen
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-05-31 21:47 PDT by Geoffrey Garen
Modified: 2016-06-01 12:31 PDT (History)
7 users (show)

See Also:

Attachments
Patch (5.35 KB, patch)
2016-05-31 21:53 PDT, Geoffrey Garen 		no flags Details | Formatted Diff | Diff
Patch (4.41 KB, patch)
2016-05-31 22:00 PDT, Geoffrey Garen 		keith_miller: review+
Details | Formatted Diff | Diff
Show Obsolete (1) 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 Geoffrey Garen 2016-05-31 21:47:27 PDT 
Dictionary property access should be fast
Comment 1 Geoffrey Garen 2016-05-31 21:53:43 PDT 
Created attachment 280212 [details]
Patch
Comment 2 Geoffrey Garen 2016-05-31 22:00:25 PDT 
Created attachment 280213 [details]
Patch
Comment 3 Keith Miller 2016-06-01 09:36:24 PDT 
Comment on attachment 280213 [details]
Patch

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

r=me with comments.

> Source/JavaScriptCore/runtime/JSObject.cpp:2004
> -    if (structure()->isDictionary()) {
> +    if (structure()->isUncacheableDictionary()) {

I think it would be useful to have a comment stating that JSObject::putDirectInternal will transition the structure on an attribute change. Also, do we have tests for this?

> Source/JavaScriptCore/runtime/JSObject.h:1228
> -    if (structure.isDictionary()) {
> +    if (structure.isUncacheableDictionary()) {

Ditto.
Comment 4 Geoffrey Garen 2016-06-01 12:28:28 PDT 
> > Source/JavaScriptCore/runtime/JSObject.cpp:2004
> > -    if (structure()->isDictionary()) {
> > +    if (structure()->isUncacheableDictionary()) {
> 
> I think it would be useful to have a comment stating that
> JSObject::putDirectInternal will transition the structure on an attribute
> change. 

Okeedokee.

> Also, do we have tests for this?

I checked SVN history and it looks like the change to putDirect corresponded to some web standards test suites.
Comment 5 Geoffrey Garen 2016-06-01 12:31:15 PDT 
Committed r201562: <http://trac.webkit.org/changeset/201562>