Bug 123047 – Spread operator should be performing direct "puts" and not triggering setters

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Oliver Hunt]

Spread operator should be performing direct "puts" and not triggering setters

[Oliver Hunt]

Created attachment 214614 [details]
Patch

[Geoffrey Garen]

Comment on attachment 214614 [details]
Patch

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

r=me

> Source/JavaScriptCore/ChangeLog:8
> +        Add a new opcode -- op_direct_put_by_value -- and make use of it in the spread

Can we call this put_by_value_direct to match put_by_id_direct?

[Oliver Hunt]

Committed r157656: <http://trac.webkit.org/changeset/157656>

[Filip Pizlo]

Comment on attachment 214614 [details]
Patch

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

Can you please de-duplicate some of this code?

> Source/JavaScriptCore/jit/JITOperations.cpp:598
> +void JIT_OPERATION operationDirectPutByVal(ExecState* callFrame, EncodedJSValue encodedBaseValue, EncodedJSValue encodedSubscript, EncodedJSValue encodedValue)
> +{
> +    VM& vm = callFrame->vm();
> +    NativeCallFrameTracer tracer(&vm, callFrame);
> +    
> +    JSValue baseValue = JSValue::decode(encodedBaseValue);
> +    JSValue subscript = JSValue::decode(encodedSubscript);
> +    JSValue value = JSValue::decode(encodedValue);
> +    RELEASE_ASSERT(baseValue.isObject());
> +    JSObject* object = asObject(baseValue);
> +    if (subscript.isInt32()) {
> +        // See if it's worth optimizing at all.
> +        bool didOptimize = false;
> +        
> +        unsigned bytecodeOffset = callFrame->locationAsBytecodeOffset();
> +        ASSERT(bytecodeOffset);
> +        ByValInfo& byValInfo = callFrame->codeBlock()->getByValInfo(bytecodeOffset - 1);
> +        ASSERT(!byValInfo.stubRoutine);
> +        
> +        if (hasOptimizableIndexing(object->structure())) {
> +            // Attempt to optimize.
> +            JITArrayMode arrayMode = jitArrayModeForStructure(object->structure());
> +            if (arrayMode != byValInfo.arrayMode) {
> +                JIT::compileDirectPutByVal(&vm, callFrame->codeBlock(), &byValInfo, ReturnAddressPtr(OUR_RETURN_ADDRESS), arrayMode);
> +                didOptimize = true;
> +            }
> +        }
> +        
> +        if (!didOptimize) {
> +            // If we take slow path more than 10 times without patching then make sure we
> +            // never make that mistake again. Or, if we failed to patch and we have some object
> +            // that intercepts indexed get, then don't even wait until 10 times. For cases
> +            // where we see non-index-intercepting objects, this gives 10 iterations worth of
> +            // opportunity for us to observe that the get_by_val may be polymorphic.
> +            if (++byValInfo.slowPathCount >= 10
> +                || object->structure()->typeInfo().interceptsGetOwnPropertySlotByIndexEvenWhenLengthIsNotZero()) {

Is this really a copy-paste job?  That's not cool, man.