buffer code never invoked - and wrong?

[ghost]

In your code in the function matchSameProto you check for isBuffer AFTER this check

if (arrayBufferSupport !== ArrayBufferNone) {

Because of this, isBuffer is never invoked, but will still work! But slower performance (ran benchmark). You have to put the isBuffer check before that function.

Further, your isBuffer code seems to be wrong.

if (isBuffer(a)) {
        return matchView(a, b);
    }

// should be
if (isBuffer(a)) {
        return matchView(a.buffer, b.buffer);
    }

After this fixed, you will get this benchmark result:

 x 6,289,942 ops/sec ±2.48% (88 runs sampled)

[ghost]

Two buffers shiuld not be equal either, but they are. Two different instances are never equal.

[dead-claudia]

Buffers are indexed collections. a.buffer and b.buffer in newer Node
return the underlying ArrayBuffer, which cannot be indexed normally. Node
pre-4 (e.g. io.js) don't even have this property at all, since it's put
there by the internal TypedArray constructor.

The reason why it appears so much faster is because arrayBuffer[i] and
arrayBuffer.length don't normally exist. Here's what engines figure out
from an ArrayBuffer type:

function matchView(a, b) {
    var count = a.length

    if (count !== b.length) { // always false: `undefined === undefined`
        return false
    }

    while (count) {
        count--
        if (!floatIs(a[count], b[count])) return false
        // Inlined:
        // var a1 = a[count]
        // var b1 = b[count]
        // if (!(a1 === b1 || // always true: `undefined === undefined`
        //         a1 !== a1 && b1 !== b1)) {
        //     return false // never taken: `undefined === undefined`
        // }
    }

    return true
}

Engines optimizing for those types are going to do an IC type check, and
with that IC, they'll pretty much optimize it to this:

// Use `ArrayBuffer` map for own properties, remove statically known dead
code
function matchView(a, b) {
    if (!isUnchangedArrayBuffer(a) || !isUnchangedArrayBuffer(b)) {
        return recompile()
    }

    var count = a.length
    b.length

    while (count) {
        count--
        a[count]
        b[count]
    }

    return true
}

// Remove unused properties that aren't connected to getters
function matchView(a, b) {
    if (!isUnchangedArrayBuffer(a) || !isUnchangedArrayBuffer(b)) {
        return recompile()
    }

    var count = a.length

    while (count) {
        count--
    }

    return true
}

// Remove unused arithmetic from loops and branches


// Remove unused properties that aren't connected to getters
function matchView(a, b) {
    if (!isUnchangedArrayBuffer(a) || !isUnchangedArrayBuffer(b)) {
        return recompile()
    }

    a.length
    return true
}

// Remove unused properties that aren't connected to getters
function matchView(a, b) {
    if (!isUnchangedArrayBuffer(a) || !isUnchangedArrayBuffer(b)) {
        return recompile()
    }
    return true
}

So in reality, the isUnchangedArrayBuffer function is very cheap to call
(and is usually a single bytecode instruction, like CheckMaps in V8), and
the rest is independent of a or b. So this is likely to compile to
something like this hypothetical pseudo-assembly (likely inlined):

.matchView_inlined
    push rdi ; a, first argument
    call isUnchangedArrayBuffer
    jz .param1
    j .recompile

.matchView_inlined
    pop rsi ; b, second argument
    call isUnchangedArrayBuffer
    jnz .ret

.matchView_recompile
    push matchView_impl
    call recompileAndCall

.matchView_inlined_ret
    movl rax, 0 ; false

That's why that turns out so fast. Engines use ICs to narrow types down
speculatively, and use that to reduce the amount of computation required.
Removing dead and redundant code in basic cases like these is relatively
straightforward once you know types.

---

Isiah Meadows
me@isiahmeadows.com

On Tue, Sep 27, 2016 at 9:57 AM, zubuzon notifications@github.com wrote:

Two buffers shiuld not be equal either, but they are. Two different
instances are never equal.

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
#34 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AERrBEidQxXz_26T76VejPTN25gcCHVkks5quSDjgaJpZM4KHqR3
.

[ghost]

If you got a isView check first, isBuffer will never be invoked. Anyway. I'm closing this, it's a question and not an issue.

[dead-claudia]

@zubuzon I'll clarify that isBuffer will be called for Buffers in Node pre-4 and in browser polyfills (e.g. the standard Browserify one) that don't subclass Uint8Array. Anyways, I'll leave it closed.