Changeset 244313 in webkit


Ignore:
Timestamp:
Apr 15, 2019 5:28:47 PM (5 years ago)
Author:
rmorisset@apple.com
Message:

DFG should be able to constant fold Object.create() with a constant prototype operand
https://bugs.webkit.org/show_bug.cgi?id=196886

Reviewed by Yusuke Suzuki.

JSTests:

Note that this new benchmark does not currently see a speedup with inlining removed.
The reason is that we do not yet have inline caching for Object.create(), we only optimize it when the DFG can see statically the prototype being passed.

  • microbenchmarks/object-create-constant-prototype.js: Added.

(test):

Source/JavaScriptCore:

It is a fairly simple and limited patch, as it only works when the DFG can prove the exact object used as prototype.
But when it applies it can be a significant win:

Baseline Optim

object-create-constant-prototype 3.6082+-0.0979 1.6947+-0.0756 definitely 2.1292x faster
object-create-null 11.4492+-0.2510 ? 11.5030+-0.2402 ?
object-create-unknown-object-prototype 15.6067+-0.1851 ? 15.7500+-0.2322 ?
object-create-untyped-prototype 8.8873+-0.1240 ? 8.9806+-0.1202 ? might be 1.0105x slower
<geometric> 8.6967+-0.1208 7.2408+-0.1367 definitely 1.2011x faster

The only subtlety is that we need to to access the StructureCache concurrently from the compiler thread (see https://bugs.webkit.org/show_bug.cgi?id=186199)
I solved this with a simple lock, taken when the compiler thread tries to read it, and when the main thread tries to modify it.
I expect it to be extremely low contention, but will watch the bots just in case.
The lock is taken neither when the main thread is only reading the cache (it has no-one to race with), nor when the GC purges it of dead entries (it does not free anything while a compiler thread is in the middle of a phase).

  • dfg/DFGAbstractInterpreterInlines.h:

(JSC::DFG::AbstractInterpreter<AbstractStateType>::executeEffects):

  • dfg/DFGConstantFoldingPhase.cpp:

(JSC::DFG::ConstantFoldingPhase::foldConstants):

  • runtime/StructureCache.cpp:

(JSC::StructureCache::createEmptyStructure):
(JSC::StructureCache::tryEmptyObjectStructureForPrototypeFromCompilerThread):

  • runtime/StructureCache.h:
Location:
trunk
Files:
1 added
7 edited

Legend:

Unmodified
Added
Removed
  • trunk/JSTests/ChangeLog

    r244295 r244313  
     12019-04-15  Robin Morisset  <rmorisset@apple.com>
     2
     3        DFG should be able to constant fold Object.create() with a constant prototype operand
     4        https://bugs.webkit.org/show_bug.cgi?id=196886
     5
     6        Reviewed by Yusuke Suzuki.
     7
     8        Note that this new benchmark does not currently see a speedup with inlining removed.
     9        The reason is that we do not yet have inline caching for Object.create(), we only optimize it when the DFG can see statically the prototype being passed.
     10
     11        * microbenchmarks/object-create-constant-prototype.js: Added.
     12        (test):
     13
    1142019-04-15  Tadeu Zagallo  <tzagallo@apple.com>
    215
  • trunk/JSTests/stress/object-create-undefined.js

    r232442 r244313  
    2525    }, `TypeError: Object prototype may only be an Object or null.`);
    2626}
     27for (var i = 0; i < 1e4; ++i) {
     28    // Some folding does not happen in the non-inlined version, so this can test a different path through the compiler
     29    // than the previous loop.
     30    shouldThrow(() => {
     31        Object.create(undefined);
     32    }, `TypeError: Object prototype may only be an Object or null.`);
     33}
  • trunk/Source/JavaScriptCore/ChangeLog

    r244312 r244313  
     12019-04-15  Robin Morisset  <rmorisset@apple.com>
     2
     3        DFG should be able to constant fold Object.create() with a constant prototype operand
     4        https://bugs.webkit.org/show_bug.cgi?id=196886
     5
     6        Reviewed by Yusuke Suzuki.
     7
     8
     9        It is a fairly simple and limited patch, as it only works when the DFG can prove the exact object used as prototype.
     10        But when it applies it can be a significant win:
     11                                                        Baseline                   Optim                                       
     12        object-create-constant-prototype              3.6082+-0.0979     ^      1.6947+-0.0756        ^ definitely 2.1292x faster
     13        object-create-null                           11.4492+-0.2510     ?     11.5030+-0.2402        ?
     14        object-create-unknown-object-prototype       15.6067+-0.1851     ?     15.7500+-0.2322        ?
     15        object-create-untyped-prototype               8.8873+-0.1240     ?      8.9806+-0.1202        ? might be 1.0105x slower
     16        <geometric>                                   8.6967+-0.1208     ^      7.2408+-0.1367        ^ definitely 1.2011x faster
     17
     18        The only subtlety is that we need to to access the StructureCache concurrently from the compiler thread (see https://bugs.webkit.org/show_bug.cgi?id=186199)
     19        I solved this with a simple lock, taken when the compiler thread tries to read it, and when the main thread tries to modify it.
     20        I expect it to be extremely low contention, but will watch the bots just in case.
     21        The lock is taken neither when the main thread is only reading the cache (it has no-one to race with), nor when the GC purges it of dead entries (it does not free anything while a compiler thread is in the middle of a phase).
     22
     23        * dfg/DFGAbstractInterpreterInlines.h:
     24        (JSC::DFG::AbstractInterpreter<AbstractStateType>::executeEffects):
     25        * dfg/DFGConstantFoldingPhase.cpp:
     26        (JSC::DFG::ConstantFoldingPhase::foldConstants):
     27        * runtime/StructureCache.cpp:
     28        (JSC::StructureCache::createEmptyStructure):
     29        (JSC::StructureCache::tryEmptyObjectStructureForPrototypeFromCompilerThread):
     30        * runtime/StructureCache.h:
     31
    1322019-04-15  Devin Rousso  <drousso@apple.com>
    233
  • trunk/Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h

    r244193 r244313  
    11/*
    2  * Copyright (C) 2013-2018 Apple Inc. All rights reserved.
     2 * Copyright (C) 2013-2019 Apple Inc. All rights reserved.
    33 *
    44 * Redistribution and use in source and binary forms, with or without
     
    4444#include "PutByIdStatus.h"
    4545#include "StringObject.h"
     46#include "StructureCache.h"
    4647#include "StructureRareDataInlines.h"
    4748#include <wtf/BooleanLattice.h>
     
    25872588    case ObjectCreate: {
    25882589        if (JSValue base = forNode(node->child1()).m_value) {
    2589             if (base.isNull()) {
    2590                 JSGlobalObject* globalObject = m_graph.globalObjectFor(node->origin.semantic);
     2590            JSGlobalObject* globalObject = m_graph.globalObjectFor(node->origin.semantic);
     2591            Structure* structure = nullptr;
     2592            if (base.isNull())
     2593                structure = globalObject->nullPrototypeObjectStructure();
     2594            else if (base.isObject())
     2595                structure = globalObject->vm().structureCache.emptyObjectStructureConcurrently(globalObject, base.getObject(), JSFinalObject::defaultInlineCapacity());
     2596           
     2597            if (structure) {
    25912598                m_state.setFoundConstants(true);
    25922599                if (node->child1().useKind() == UntypedUse)
    25932600                    didFoldClobberWorld();
    2594                 setForNode(node, globalObject->nullPrototypeObjectStructure());
    2595                 break;
    2596             }
    2597             // FIXME: We should get a structure for a constant prototype. We need to allow concurrent
    2598             // access to StructureCache from compiler threads.
    2599             // https://bugs.webkit.org/show_bug.cgi?id=186199
     2601                setForNode(node, structure);
     2602                break;
     2603            }
    26002604        }
    26012605        if (node->child1().useKind() == UntypedUse)
  • trunk/Source/JavaScriptCore/dfg/DFGConstantFoldingPhase.cpp

    r244067 r244313  
    11/*
    2  * Copyright (C) 2012-2018 Apple Inc. All rights reserved.
     2 * Copyright (C) 2012-2019 Apple Inc. All rights reserved.
    33 *
    44 * Redistribution and use in source and binary forms, with or without
     
    4040#include "JSCInlines.h"
    4141#include "PutByIdStatus.h"
     42#include "StructureCache.h"
    4243
    4344namespace JSC { namespace DFG {
     
    751752            case ObjectCreate: {
    752753                if (JSValue base = m_state.forNode(node->child1()).m_value) {
    753                     if (base.isNull()) {
    754                         JSGlobalObject* globalObject = m_graph.globalObjectFor(node->origin.semantic);
    755                         node->convertToNewObject(m_graph.registerStructure(globalObject->nullPrototypeObjectStructure()));
     754                    JSGlobalObject* globalObject = m_graph.globalObjectFor(node->origin.semantic);
     755                    Structure* structure = nullptr;
     756                    if (base.isNull())
     757                        structure = globalObject->nullPrototypeObjectStructure();
     758                    else if (base.isObject())
     759                        structure = globalObject->vm().structureCache.emptyObjectStructureConcurrently(globalObject, base.getObject(), JSFinalObject::defaultInlineCapacity());
     760                   
     761                    if (structure) {
     762                        node->convertToNewObject(m_graph.registerStructure(structure));
    756763                        changed = true;
    757764                        break;
    758765                    }
    759                     // FIXME: We should get a structure for a constant prototype. We need to allow concurrent
    760                     // access to StructureCache from compiler threads.
    761                     // https://bugs.webkit.org/show_bug.cgi?id=186199
    762766                }
    763767                break;
  • trunk/Source/JavaScriptCore/runtime/StructureCache.cpp

    r232337 r244313  
    11/*
    2  * Copyright (C) 2013-2017 Apple Inc. All rights reserved.
     2 * Copyright (C) 2013-2019 Apple Inc. All rights reserved.
    33 *
    44 * Redistribution and use in source and binary forms, with or without
     
    2727#include "StructureCache.h"
    2828
    29 #include "IndexingType.h"
    3029#include "JSGlobalObject.h"
    3130#include "JSCInlines.h"
     31#include <wtf/Locker.h>
    3232
    3333namespace JSC {
     
    3737    RELEASE_ASSERT(!!prototype); // We use nullptr inside the HashMap for prototype to mean poly proto, so user's of this API must provide non-null prototypes.
    3838
     39    // We don't need to lock here because only the main thread can get here, and only the main thread can mutate the cache
    3940    PrototypeKey key { makePolyProtoStructure ? nullptr : prototype, executable, inlineCapacity, classInfo, globalObject };
    4041    if (Structure* structure = m_structures.get(key)) {
     
    5960            vm, globalObject, prototype, typeInfo, classInfo, indexingType, inlineCapacity);
    6061    }
     62    auto locker = holdLock(m_lock);
    6163    m_structures.set(key, structure);
     64    return structure;
     65}
    6266
    63     return structure;
     67Structure* StructureCache::emptyObjectStructureConcurrently(JSGlobalObject* globalObject, JSObject* prototype, unsigned inlineCapacity)
     68{
     69    RELEASE_ASSERT(!!prototype); // We use nullptr inside the HashMap for prototype to mean poly proto, so user's of this API must provide non-null prototypes.
     70   
     71    PrototypeKey key { prototype, nullptr, inlineCapacity, JSFinalObject::info(), globalObject };
     72    auto locker = holdLock(m_lock);
     73    if (Structure* structure = m_structures.get(key)) {
     74        ASSERT(prototype->mayBePrototype());
     75        return structure;
     76    }
     77    return nullptr;
    6478}
    6579
  • trunk/Source/JavaScriptCore/runtime/StructureCache.h

    r229161 r244313  
    11/*
    2  * Copyright (C) 2013-2017 Apple Inc. All rights reserved.
     2 * Copyright (C) 2013-2019 Apple Inc. All rights reserved.
    33 *
    44 * Redistribution and use in source and binary forms, with or without
     
    3030#include "PrototypeKey.h"
    3131#include "WeakGCMap.h"
     32#include <wtf/Lock.h>
    3233#include <wtf/TriState.h>
    3334
     
    5253    JS_EXPORT_PRIVATE Structure* emptyObjectStructureForPrototype(JSGlobalObject*, JSObject*, unsigned inlineCapacity, bool makePolyProtoStructure = false, FunctionExecutable* = nullptr);
    5354    JS_EXPORT_PRIVATE Structure* emptyStructureForPrototypeFromBaseStructure(JSGlobalObject*, JSObject*, Structure*);
     55    JS_EXPORT_PRIVATE Structure* emptyObjectStructureConcurrently(JSGlobalObject*, JSObject* prototype, unsigned inlineCapacity);
    5456
    5557private:
     
    5860    using StructureMap = WeakGCMap<PrototypeKey, Structure>;
    5961    StructureMap m_structures;
     62    Lock m_lock;
    6063};
    6164
Note: See TracChangeset for help on using the changeset viewer.