microsoft / tslib

Runtime library for TypeScript helpers.
BSD Zero Clause License
1.25k stars 126 forks source link

__createBinding performance issue on web browsers #165

Closed jramsley closed 2 years ago

jramsley commented 2 years ago

__createBinding, which is used in __exportStar, uses accessor descriptors to get exported modules. In our product, we see a 10x performance degradation referencing exported enums this way compared to using a direct access object. We've verified that by refactoring our enum exports to be explicitly defined and not *, this issue is not seen.

Likewise, this issue is not seen in v1.14.0 when __createBinding looks like

    __createBinding = function(o, m, k, k2) {
        if (k2 === undefined) k2 = k;
        o[k2] = m[k];
    };

current implementation:

    __createBinding = Object.create ? (function(o, m, k, k2) {
        if (k2 === undefined) k2 = k;
        Object.defineProperty(o, k2, { enumerable: true, get: function() { return m[k]; } });
    }) : (function(o, m, k, k2) {
        if (k2 === undefined) k2 = k;
        o[k2] = m[k];
    });

JSBench: https://jsbench.me/yokwmk1mau/1

jakebailey commented 2 years ago

With #168 merged, is this issue fixed? @jramsley can you still reproduce this in the latest tslib?

jramsley commented 2 years ago

It helps, but it is still tangibly slower using descriptors than accessing the object directly. It is still a 2x performance degradation compared to overwriting tslib with

    __createBinding = function(o, m, k, k2) {
        if (k2 === undefined) k2 = k;
        o[k2] = m[k];
    };
ExE-Boss commented 2 years ago

@jramsley

    __createBinding = function(o, m, k, k2) {
        if (k2 === undefined) k2 = k;
        o[k2] = m[k];
    };

The issue with that is that if m[k] is later mutated (or is a getter), the mutation won’t propagate when accessing o[k2], like it does in the case of real ES modules.

jakebailey commented 2 years ago

Yeah, that's the case I was trying to remember (thanks for remembering). One can write:

// @filename: /mod.ts
export let foo = 1234;

// @filename: /index.ts
import { foo } from "./mod"

And foo in the second module will change when mod.ts changes its value.

jramsley commented 2 years ago

The issue with that is that if m[k] is later mutated (or is a getter), the mutation won’t propagate when accessing o[k2], like it does in the case of real ES modules.

This is correct, but for our use case the * import is for classes and enums which aren't being mutated.

jakebailey commented 2 years ago

Unfortunately, I don't think TypeScript is going to be able to provide the info required to prove that we can make that optimization; if you're doing export * from "./otherFile" in some file, it can't know which of those bindings are mutable and which aren't (and that can continue onward as it's reexported). I think that this is about as fast as we're going to be able to make it.