Immer: "Performance" section of ReadMe is very misleading

[cakoose]

The "Performance" section of the ReadMe contains the results for a single benchmark scenario, along with this observation:

Immer is roughly as fast as ImmutableJS. [...]

For these kinds of libraries, different scenarios will produce wildly different results. Including just this single chart is very misleading.

It would be a huge improvement just to include one additional scenario: modifying a single TODO list item. That might help readers understand the tradeoffs. (In particular, I expect a persistent data structure library ImmutableJS to be much, much faster at that.)

[mweststrate]

Please don't use the label "bug" for things like this.

Benchmarks always take some artificial scenario that is not representative for every possible scenario. The bottom line of the benchmarks is: For all practical purposes immer won't be a bottle neck if immutablejs wasn't. Which so far seems to be quite true given the amount of performance issues filed by our thousands of users: 0.

Anyway, maybe you are right, maybe you are wrong. But please first show a benchmark with numbers. The readme might be misleading, but is at least grounded in numbers, something that cannot be set about this issue, and your expectations might very well be suprised :).

So please provide a real benchmark, and we can continue the discussion based on that (whether that is a better benchmark etc etc). Otherwise we are just discussing your assumptions which is quite pointless :).

[cakoose]

(Sorry about filing as "Bug". I saw "Bug" and "Feature Request" as the two options and neither really fit so I arbitrarily picked "Bug". I now see that there's a "Open a regular issue" fallback option at the bottom.)

And yes, implicit in my initial report is that ImmutableJS performs much better in certain situations (e.g. modifying a single TODO list item). I thought that was non-controversial given the different designs, but I guess not. Whoops.

Running the existing Immer benchmark (Node 12.1.0, MacOS), I get:

# todo - performance

...
immutableJS: 6ms
immutableJS + toJS: 77ms
seamless-immutable: 52ms
seamless-immutable + asMutable: 95ms
immer (proxy) - without autofreeze: 36ms
immer (proxy) - with autofreeze: 30ms
immer (proxy) - without autofreeze - with patch listener: 26ms
immer (proxy) - with autofreeze - with patch listener: 28ms
...

# incremental - lot of small incremental changes

...
immutableJS: 7ms
immutableJS - batched mutation: 2ms   # <-- I added this
immer (proxy): 182ms
immer (es5): 1112ms
immer (proxy) - single produce: 4ms
immer (es5) - single produce: 2ms

This is strange... the chart in the ReadMe shows Immer and ImmutableJS being roughly the same, while these results show ImmutableJS being much faster. Is the chart not based off the "todo" example? Or maybe it was generated with a different Node version or ImmutableJS version?

(Other than adding the "immutableJS - batched mutation" benchmark, I didn't change any of the code.)

[mweststrate]

My results are:

# add-data - loading large set of data

just mutate: 0ms
just mutate, freeze: 0ms
handcrafted reducer (no freeze): 0ms
handcrafted reducer (with freeze): 0ms
immutableJS: 49ms
immutableJS + toJS: 47ms
seamless-immutable: 52ms
seamless-immutable + asMutable: 53ms
immer (proxy) - without autofreeze: 0ms
immer (proxy) - with autofreeze: 0ms
immer (es5) - without autofreeze: 0ms
immer (es5) - with autofreeze: 0ms

# todo - performance

just mutate: 1ms
just mutate, freeze: 232ms
deepclone, then mutate: 147ms
deepclone, then mutate, then freeze: 578ms
handcrafted reducer (no freeze): 8ms
handcrafted reducer (with freeze): 26ms
naive handcrafted reducer (without freeze): 59ms
naive handcrafted reducer (with freeze): 42ms
immutableJS: 55ms
immutableJS + toJS: 94ms
seamless-immutable: 87ms
seamless-immutable + asMutable: 166ms
immer (proxy) - without autofreeze: 25ms
immer (proxy) - with autofreeze: 52ms
immer (proxy) - without autofreeze - with patch listener: 41ms
immer (proxy) - with autofreeze - with patch listener: 42ms
immer (es5) - without autofreeze: 217ms
immer (es5) - with autofreeze: 145ms
immer (es5) - without autofreeze - with patch listener: 170ms
immer (es5) - with autofreeze - with patch listener: 155ms

# incremental - lot of small incremental changes

just mutate: 0ms
handcrafted reducer: 52ms
immutableJS: 3ms
immer (proxy): 320ms
immer (es5): 1746ms
immer (proxy) - single produce: 5ms
immer (es5) - single produce: 3ms
Done in 34.28s.

Obviously batching will make immutableJS faster, but the same applies for immer (doing a lot of mutations in a single produce call), so adding a batched mutation example doesn't really change much imho. That is actually what the todo benchmark is doing.

Anyway I think immer has proven to be fast enough compared to immutable in practice. Especially since for a proper real life comparison with immutableJS one should probably incorporate the back-to-JS conversion which is expensive.

I do understand where your question is coming from, the immutablejs TRIE structure should in theory allow for faster insertions, but in practice I think the difference doesn't matter for 99,8% of the applications, so I rather continue this discussion when a real life bottleneck with immer arises, before concluding that the benchmark is "very misleading".

As said, endless jack-shaving on what would make up to be the best benchmark is possible, and sure there is always some benchmark imaginable in which one solution prevails over the other. The general take-away from the current benchmark should be: don't worry about immer's performance. And that remains the case.

[cakoose]

_I'm listing my remaining concerns in the hope that they're useful. But this is your project so feel free to ignore them and re-close the bug._

What platform are you running on (Node version, OS, CPU)? I'm curious why your "todo" benchmark results are so different from mine.

Obviously batching will make immutableJS faster, but the same applies for immer (doing a lot of mutations in a single produce call), so adding a batched mutation example doesn't really change much imho.

The only reason I added the batched mutation variant for ImmutableJS because there was a batched variant for Immer ("single produce").

The more interesting numbers for the "incremental" benchmark are without batching, where ImmutableJS is ~100x faster. Why not mention these results in the ReadMe?

Especially since for a proper real life comparison with immutableJS one should probably incorporate the back-to-JS conversion which is expensive.

No it shouldn't. Anyone choosing to use ImmutableJS will not convert back to JS after every operation. That would 100% defeat the purpose of using ImmutableJS.

It's useful to include the back-to-JS results somewhere, so people are aware, but it shouldn't be the primary number.

As said, endless jack-shaving on what would make up to be the best benchmark is possible, and sure there is always some benchmark imaginable in which one solution prevails over the other.

Yes, endless discussion is possible, but that's not what I'm after. I brought up a specific use case that I think is worth including in the "Performance" section. Whoever wrote the "incremental" benchmark also seemed to think that use case is worth measuring.

And at first glance, the "todo" benchmark is strange. When would you be modifying 10% of your TODO list items in a single step? Isn't it more common to modify a single one? And when you do that, ImmutableJS is much faster.

[mweststrate]

Specs:

$ node -v && uname -a
v10.15.1
Linux x250 5.0.0-16-generic #17-Ubuntu SMP Wed May 15 10:52:21 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux

The more interesting numbers for the "incremental" benchmark are without batching, where ImmutableJS is ~100x faster. Why not mention these results in the ReadMe?

Isn't it more common to modify a single one?

The idea is this: if the mutation is really small, it doesn't really matter how expensive it is. 0.001 ms or 1ms are both irrelevant from perf perspective, even though it is 1000 times slower. It can only hurt when many things are done, (or the same thing is repeated many times) which amplifies these numbers. This is why many todos are changed in the benchmark. (real life use case: merging in a freshly received data set, or a complex operation that touches many pieces of the state, such as "apply a discount to all order larger than 10 $")

convert back to JS after every operation

The context is mostly: in client side apps (which is afaik the most common use case of immer), you don't need to convert for the operation, but you probably do need to convert parts for the rendering. Which is probably only subset of the data set, but still can be a price to pay. I heard quite some people having trouble with it, which is why I thought this number to be important as well.

[cakoose]

The idea is this: if the mutation is really small, it doesn't really matter how expensive it is. 0.001 ms or 1ms are both irrelevant from perf perspective, even though it is 1000 times slower. It can only hurt when many things are done, (or the same thing is repeated many times) which amplifies these numbers.

Ah, I think I now understand where you're coming from.

I agree that there are plenty of use cases where what you're saying is true. Roughly:

• Each V8 isolate is only handling a single application.
• The host machine has plenty of spare CPU, so overall efficiency doesn't matter.

Mine is a server-side use case.

• Each V8 isolate is serving many concurrent requests, so the difference between 1ms event loop blockage and 0.001ms event loop blockage matters since all you have is a single thread.
• The host machine's CPUs aren't ever sitting idle, so if mutation operations use 100x less CPU, that matters.
• We have large heaps and are running into issues with GC (high CPU usage, long pauses), so reducing allocations is important.

Perhaps the ideal use cases for Immer are obvious to some, but think about people who run into the project for the first time. For example, I was just searching around for persistent data structure libraries and got the impression that Immer was similar to ImmutableJS but with a friendlier API. I had to dig in quite a bit to find out that wasn't exactly the case.

I think having two data points in the "Performance" section (big update and small update) will go a long way towards giving a correct first impression. It can be accompanied by your explanation for why 0.001ms vs 1ms often doesn't matter, and readers can decide if that holds true for their use case.

v10.15.1
Linux x250 5.0.0-16-generic #17-Ubuntu SMP Wed May 15 10:52:21 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux

This is strange. I'm getting much better performance for ImmutableJS on my Mac laptop and on a GCP Linux instance:

Mac laptop:

$ node --version && uname -a
v10.15.1
Darwin kannan 18.5.0 Darwin Kernel Version 18.5.0: Mon Mar 11 20:40:32 PDT 2019; root:xnu-4903.251.3~3/RELEASE_X86_64 x86_64

# todo - performance

just mutate: 0ms
deepclone, then mutate: 98ms
handcrafted reducer (no freeze): 8ms
naive handcrafted reducer (without freeze): 7ms
immutableJS: 3ms
immutableJS + toJS: 73ms
seamless-immutable: 78ms
seamless-immutable + asMutable: 144ms
immer (proxy) - without autofreeze: 19ms
immer (proxy) - without autofreeze - with patch listener: 27ms

# incremental - lot of small incremental changes

just mutate: 1ms
handcrafted reducer: 46ms
immutableJS: 7ms
immer (proxy): 221ms
immer (es5): 1096ms



md5-d3eca1e79dd0eaa8d03bdaf05207236e



$ node --version && uname -a
v10.15.2
Linux instance-2 5.0.0-1008-gcp #8-Ubuntu SMP Wed Jun 5 09:00:32 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux



md5-2b584e0873e4f5407b615a0f3d87b5f9



# todo - performance

just mutate: 1ms
deepclone, then mutate: 202ms
handcrafted reducer (no freeze): 30ms
naive handcrafted reducer (without freeze): 8ms
immutableJS: 5ms
immutableJS + toJS: 145ms
immer (proxy) - without autofreeze: 33ms
immer (proxy) - without autofreeze - with patch listener: 51ms

# incremental - lot of small incremental changes

just mutate: 0ms
handcrafted reducer: 64ms
immutableJS: 3ms
immer (proxy): 307ms
immer (es5): 1777ms

This is off a clean "git clone", "yarn install", and "yarn run test:perf".

My "yarn.lock" has "[email protected]". Is that what you have as well?