Bug 170881 - REGRESSION(215272): microbenchmark/seal-and-do-work and microbenchmark/freeze-and-do-work are 27x slower
Summary: REGRESSION(215272): microbenchmark/seal-and-do-work and microbenchmark/freeze...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Michael Saboff
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2017-04-15 21:08 PDT by Caio Lima
Modified: 2017-04-18 10:54 PDT (History)
8 users (show)

See Also:


Attachments
Patch (1.90 KB, patch)
2017-04-17 16:36 PDT, Michael Saboff
sbarati: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Caio Lima 2017-04-15 21:08:47 PDT
Comparing r215126 with r215394 I've found the following performance regression into microbenchmarks. Need to investigate the reason.

                             baseline                  changes                                      

freeze-and-do-work       17.8163+-1.1731     !    481.5174+-10.8908       ! definitely 27.0268x slower
seal-and-do-work         18.0572+-1.2018     !    491.9421+-18.0567       ! definitely 27.2436x slower

<geometric>              17.9014+-0.9676     !    486.4490+-11.4703       ! definitely 27.1737x slower
Comment 1 Michael Saboff 2017-04-16 10:20:22 PDT
This is like change set r215272 <https://trac.webkit.org/changeset/215272>.  I'll take a look at it.
Comment 2 Radar WebKit Bug Importer 2017-04-16 13:25:28 PDT
<rdar://problem/31649374>
Comment 3 Michael Saboff 2017-04-17 16:36:07 PDT
Created attachment 307319 [details]
Patch
Comment 4 Saam Barati 2017-04-17 17:09:42 PDT
Comment on attachment 307319 [details]
Patch

r=me
Comment 5 Mark Lam 2017-04-17 17:17:38 PDT
Comment on attachment 307319 [details]
Patch

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

> Source/JavaScriptCore/runtime/ObjectConstructor.cpp:614
> +    if (isJSFinalObject(object) && !hasIndexedProperties(object->indexingType())) {
> +        object->seal(vm);
> +        return JSValue::encode(obj);
> +    }

In r215272 <https://trac.webkit.org/changeset/215272>, you also eliminated the fast paths for objectConstructorIsSealed() and objectConstructorIsFrozen().  Should those be restored as well with the additional !hasIndexedProperties() check?
Comment 6 Michael Saboff 2017-04-18 09:25:43 PDT
(In reply to Mark Lam from comment #5)
> Comment on attachment 307319 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=307319&action=review
> 
> > Source/JavaScriptCore/runtime/ObjectConstructor.cpp:614
> > +    if (isJSFinalObject(object) && !hasIndexedProperties(object->indexingType())) {
> > +        object->seal(vm);
> > +        return JSValue::encode(obj);
> > +    }
> 
> In r215272 <https://trac.webkit.org/changeset/215272>, you also eliminated
> the fast paths for objectConstructorIsSealed() and
> objectConstructorIsFrozen().  Should those be restored as well with the
> additional !hasIndexedProperties() check?

Yes.  I added those checks back in.
Comment 7 Michael Saboff 2017-04-18 10:54:28 PDT
Committed r215471: <http://trac.webkit.org/changeset/215471>