High cpu cost of Game.market.getAllOrders()

tedivm

I recorded several call of Game.market.getAllOrders() and the cpu used by it. I would say this function is far above "AVERAGE" and should be switched to "HIGH" (my pathfinding doesn't even tend to be this much).

tick #13788493: getAllOrders takes 5.493295000000003

tick #13788497: getAllOrders takes 23.142765999999995

tick #13788500: getAllOrders takes 24.875880000000024

tick #13788503: getAllOrders takes 6.346783000000002

tick #13788507: getAllOrders takes 5.8266690000000025

tick #13788510: getAllOrders takes 28.858847999999995

tick #13788515: getAllOrders takes 5.578883000000005

tick #13788521: getAllOrders takes 18.022187

tick #13788526: getAllOrders takes 18.995895000000004

tick #13788529: getAllOrders takes 5.6255350000000135

tick #13788534: getAllOrders takes 24.725386000000015

tick #13788539: getAllOrders takes 5.808078000000002

tick #13788544: getAllOrders takes 5.686706999999998

tick #13788548: getAllOrders takes 13.937843

tick #13788554: getAllOrders takes 6.007058000000001

tick #13788558: getAllOrders takes 5.841618999999998

tick #13788561: getAllOrders takes 19.773326999999995

tick #13788558: getAllOrders takes 5.841618999999998

tick #13788561: getAllOrders takes 19.773326999999995

tick #13788565: getAllOrders takes 8.456863999999996

tick #13788568: getAllOrders takes 5.882891000000001

tick #13788572: getAllOrders takes 5.796863999999999

tick #13788575: getAllOrders takes 27.224322000000008

tick #13788581: getAllOrders takes 6.444737999999997

tick #13788584: getAllOrders takes 6.359931000000003

tick #13788587: getAllOrders takes 5.867739999999998

tick #13788588: getAllOrders takes 18.782126000000005

tick #13788592: getAllOrders takes 5.692917999999992

Dissi

I'm seeing the same issue abroad:

[diagnose] - Method [GL_handleMarketData] times/tick reported: 1.00 ticks: 2699 Avg/action: 8.24

artch

Note that this high cost is charged only on the first invocation within the tick. All subsequent calls are close to zero. This is why it is marked as AVERAGE.

tedivm

I only call it once per tick, so that doesn't do anything for me. This is still a very high cost item- even pathfinding doesn't use this much CPU in most circumstances.

Consider this- calling this function is essentially impossible on the 10cpu plan, and for users at smaller GCLs this can potentially take the majority of their CPU for a tick with a single call.

Considering the data is static for each tick (as in, new orders won't be added in during a tick, just between them) it seems like there should be a way to preload this data once for all users on a thread/worker rather than forcing each user to JSON-Parse things themselves. As more people get involved in the market and the data gets larger the performance will only get worse.

ags131

Looking at sim sources it looks like its using `JSON.parse(JSON.stringify(a.market.orders))` for the cloning, JSON functions can be very slow compared to cloning withon JS. `orders = a.market.orders.map(obj=>Object.assign({}, obj))` or `_.cloneDeep` would clone all objects and should save some CPU

artch

JSON.parse(JSON.stringify(x)) is proved to be the fastest deep cloning method actually. _.cloneDeep is significantly slower. I’m curious if you have any benchmark proofs of the opposite.

tedivm

Benchmark-

https://dracoblue.net/dev/whats-faster-jsonparsestringify-or-extendmixin/

Code-

https://gist.github.com/DracoBlue/354628

artch

This blog post is 6 years old. process.mixin is not a thing anymore.

tedivm

From ags131-

https://albertxing.com/stool/#cf5439a95052408514e9d3f140057f6b

ags131

I'm getting ~300ops/sec diff between the results on my home PC (Decent I7 CPU)

artch

In Chrome I suppose? On Node.js it looks differently:

var startTime = Date.now();

for(var i=0; i<1000; i++) {
    let copy = JSON.parse(JSON.stringify(orders))
}

console.log('JSON:', Date.now() - startTime, 'ms');

startTime = Date.now();

for(var i=0; i<1000; i++) {
    let copy = {}
    let ids = Object.keys(orders)
    for(let i=0;i<ids.length;i++)
        copy[ids[i]] = Object.assign({}, orders[ids[i]])
}

console.log('Object.assign:',Date.now() - startTime,'ms');

Output:

JSON: 1820 ms
Object.assign: 2536 ms

RocketRidah

Benchmark.js with node 6.6.0 on a Win 10 64-bit with Intel i7 6700K and 32GB RAM, ran 3 times.

https://gist.github.com/mcunha/b0f14bb71d4878bd086cb8bfcf0e6cec

tedivm

Would it be possible to break it up then?

getSellOrders(resource=null)

getBuyOrders(resource=null)

and then have getAllOrders just act as a wrapper around those?

This may require changing your underlying data structure a little bit, but it would also allow people who only want a smaller subset of the data to be able to get it. Looking at sell orders for subscriptions, for instance, would be super cheap.

artch

Benchmark.js with node 6.6.0

Oh, that’s actually interesting. Looks like Object.assign has been improving in recent Node.js versions:

~# n use 5.10.1 test.js
JSON x 775 ops/sec ±4.22% (73 runs sampled)
Object assign x 621 ops/sec ±10.00% (70 runs sampled)
Fastest is JSON

~# n use 6.2.1 test.js
JSON x 744 ops/sec ±8.26% (68 runs sampled)
Object assign x 773 ops/sec ±9.23% (83 runs sampled)
Fastest is JSON,Object assign

~# n use 6.6.0 test.js
JSON x 877 ops/sec ±1.96% (81 runs sampled)
Object assign x 1,173 ops/sec ±0.73% (92 runs sampled)
Fastest is Object assign

Initially we’ve done our tests on 5.x a year ago. Anyway, Object.assign is only 30% faster, not a very big deal, and it doesn’t allow deep cloning (in case if we'll have inner objects in order info).

This may require changing your underlying data structure a little bit

Yes, internal data indexing is the only approach to speed things up which I can see here. It has its own drawbacks on our end, so we need to think about it carefully. For now we are not concerned with its current CPU cost, getAllOrders is not supposed to be called every tick.

Meanwhile, the CPU cost label for this method has been changed to HIGH.