quickjs icon indicating copy to clipboard operation
quickjs copied to clipboard

Performance issues with object property lookup

Open HarlonWang opened this issue 1 year ago • 11 comments

const a = {}
for (let i = 0; i < 10000000; i++) {
    a[i] = i
}

const start = new Date().getTime()
a[1]
console.log("cost:", (new Date().getTime() - start), "ms")

The cost time between QuickJS and V8 (Node) when running the above code. quickjs: 83 ms v8: 1 ms

Object property queries under QuickJS still take a relatively long time, I would like to understand the reason for this performance gap, and whether there are any optimization strategies. Additionally, when I changed the object to an array, I was able to save about half of the time

HarlonWang avatar Mar 01 '24 09:03 HarlonWang

https://github.com/quickjs-ng/quickjs/pull/120 ?

hxcliff avatar Mar 04 '24 09:03 hxcliff

The cost time between QuickJS and Node when running the above code. quickjs: 83 ms node: 1 ms

Which part of the code is measured? The array creation or the array element access? The code fragment seems to suggest a timing of 83ms for just a single access a[1], this is abnormal indeed. I shall investigate.

chqrlie avatar Mar 04 '24 13:03 chqrlie

The cost time between QuickJS and Node when running the above code. quickjs: 83 ms node: 1 ms

Which part of the code is measured? The array creation or the array element access? The code fragment seems to suggest a timing of 83ms for just a single access a[1], this is abnormal indeed. I shall investigate.

yes, a timing of a single access a[1], you can see the code for my time consumption statistics is printed before and after a[1].

HarlonWang avatar Mar 05 '24 10:03 HarlonWang

quickjs-ng/quickjs#120 ?

I'm not sure what you mean, but I tried running it with quickjs-ng and the results were pretty similar, even though it used an inline cache.

HarlonWang avatar Mar 05 '24 10:03 HarlonWang

FWIW, I tested it and got the same results. Even when using the v8 command line with no JIT.

saghul avatar Mar 05 '24 10:03 saghul

This looks like a gc issue or possibly a paging issue.

I get a slow timing of 29ms the first time and 0ms after that.

Edit: indeed a gc issue, see below

chqrlie avatar Mar 05 '24 14:03 chqrlie

quickjs-ng/quickjs#120 ?

I'm not sure what you mean, but I tried running it with quickjs-ng and the results were pretty similar, even though it used an inline cache.我不确定你的意思,但我尝试使用quickjs-ng运行它,结果非常相似,尽管它使用了内联缓存。

Yeah, I noticed that too. Doesn't seem relevant.

hxcliff avatar Mar 05 '24 16:03 hxcliff

I modified the test:

function t() {
    var a0 = Date.now();
    let a = {};
    for (let i = 0; i < 10000000; i++) {
        a[i] = i;
    }
    var a1 = Date.now();
    a[1];
    var a2 = Date.now();
    new Date();
    var a3 = Date.now();
    a = null;
    var a4 = Date.now();
    console.log("costs:", a1 - a0, "ms", a2 - a1, "ms", a3 - a2, "ms", a4 - a3, "ms");
}

t();

The timings are: costs: 667 ms 0 ms 30 ms 20 ms

The problem is unrelated to the object access a[1], the call to new Date() takes 30ms (which is way too long on my laptop).

new Date() creates a Date object and initializes it with the current time, which Date.now() computes the same way. The problem hence is in the object creation. I suspect the problem might be in the shape handing, possibly because the shape of the object pointed to bya is huge so rehashing the shape table might take a long time. (edit: not a problem, rehashing is efficient, as the hash key is stored in the shape, there is no need to recompute it, hence constant time for each object).

chqrlie avatar Mar 06 '24 15:03 chqrlie

While not portable, I guess testing with os.now() would remove the noise Date creates? https://github.com/bellard/quickjs/commit/2ee6be705fde0eb68acec25915d2947de1207abb

saghul avatar Mar 06 '24 15:03 saghul

While not portable, I guess testing with os.now() would remove the noise Date creates? 2ee6be7

os.now() and Date.now() give the same timings. The slow operation is new Date().

chqrlie avatar Mar 06 '24 15:03 chqrlie

I nailed it! Creating a new object ultimately calls JS_NewObjectFromShape which in turn calls js_trigger_gc(ctx->rt, sizeof(JSObject)). This is actually the only place where the gc is triggered. The huge object (10 million named properties) must be iterated as part of the gc and this takes a long time even though none of the properties are objects themselves.

This test is pathological as it constructs a huge object in one long loop without constructing any other object and allocating and reallocating a lot of memory so the next object creation triggers a gc for sure.

We could improve the gc speed by keeping a flag in each object indicating if any of the properties are (or have been) objects references, hence accelerating the scan for arrays and objects with just scalar property values (undefined, null, boolean, numbers, strings...). I am not sure how this would impact the overall performance of the interpreter (just a marginal cost when setting property values, that might be implemented without branching). It should reduce the gc overhead significantly, thereby reducing the lag it can cause in large applications.

chqrlie avatar Mar 06 '24 19:03 chqrlie