nrn / universal-copy

Deep copy anything.
ISC License
15 stars 10 forks source link

Possible performance tuning. #1

Open Raynos opened 8 years ago

Raynos commented 8 years ago

I replaced two lines in my code base from:

var copy = JSON.parse(JSON.stringify(o))

to

var copy = universalCopy(o)

And it increased the runtime of test suite from 4 seconds to 5 seconds.

My code base is a type checker, the main place i use universalCopy() is to copy the type definition files AST, which is a large data structure.

JSON.parse(JSON.stringify()):

$ time node test/index.js | grep -v '^ok' | grep -v '^# '
pid: 28829
TAP version 13
1..12275

real    0m16.841s
user    0m16.800s
sys 0m0.256s

universal-copy:

$ time node test/index.js | grep -v '^ok' | grep -v '^# '
pid: 28711
TAP version 13
1..12275

real    0m31.591s
user    0m31.464s
sys 0m0.420s

I also created two flamegraphs:

Image of unversal-copy

Also, from these flames, it looks like the other huge CPU hog is my naively implemented language parser using the parsimmon framework :)

It would be cool if benchmarks can be added and this library can be performance tuned.

Once I start performance tuning my application, I can optimize the copy() function and contribute the code back (it might make simplification trade-offs for performance... and no longer be "universal")

Raynos commented 8 years ago

Btw:

If node-flame doesn't work, then you could also play with 0x ( https://github.com/davidmarkclements/0x ) which I've never personally used.

nrn commented 8 years ago

Thanks for all the details. I'll dig into it when I can.

Off the top of my head, the FakeMap that gets used if you don't have native maps available probably isn't performant, and moveProps is costly in the name of correctness.

Trying gutting moveProps to just do a for in loop and not worry about symbols/property descriptor nonsense is the first thing to look at to sacrifice completeness for perf.