Potential bug discovered during benchmark stress tests

[transitive-bullshit]

Bug Description

I created a comparison of the most popular TS reactivity libs, including Valtio.

During my deep-dive, I wanted to benchmark all of them and created a fork of @milomg's excellent js-reactivity-benchmark, which includes dozens of benchmarks and stress tests.

The problem is that Valtio fails to complete some of the dynamic benchmark tests in the suite.

I've been unable to track down the source of the issue, whether it may be due to a rare bug in Valtio or possibly due to a bug in the Valtio adapter used by the benchmark. Note that this was a bit of an awkward adapter to write given that Valtio doesn't automatically track dependencies like many other reactive libs (#149).

How to reproduce the issue:

1. Checkout the minimal repro branch of https://github.com/transitive-bullshit/js-reactivity-benchmark and switch to the feature/minimal-issues-repro-valtio branch
2. Run pnpm install
3. Run pnpm test to make sure everything's working locally
4. Run pnpm bench to reproduce the issue

The valtio benchmark tests work fine for the first test case but then hangs on the second (slightly larger) test case. Note that these test cases are creating a rectangular grid of shallow signals, where each layer of the grid contains computed properties depending on some number of the previous row's signals as a stress test.

CPU remains at 100% for the hung test with RAM usage flat around 75MB which seems reasonable.

I've paired down this branch as much as possible to try and make it easier to debug.

Note that in the full benchmark branch, all of the other reactivity libs are able to pass these benchmark tests, except for MobX which runs into a similar issue (mobxjs/mobx#3926).

Versions

• latest [email protected]
• node v22.4.1
• macos sonoma 14.5

Reproduction Link

https://github.com/transitive-bullshit/js-reactivity-benchmark/tree/feature/minimal-issues-repro-valtio

[dai-shi]

Tried it, but hard to tell.
The adapter seems tricky, especially with computed. I'm not sure how watchGet hack works either.
I think it may require more work to add a reactive layer on top of valtio.
Can you investigate how 100% CPU is caused by? sync: true is causing an infinite loop?

[dai-shi]

I'll try my best with what's discussed in this discussion.

[dai-shi]

Here we go! https://github.com/valtiojs/valtio-reactive

[transitive-bullshit]

This looks awesome @dai-shi! Will start a new issues over in valtio-reactive to discuss. Amazing work 🔥 🙏

[dai-shi]

Nice to hear!

One note is that it might not be glitch-free, yet.

[romanlex]

@transitive-bullshit what about refreshing valtio bench in https://github.com/transitive-bullshit/ts-reactive-comparison

[dai-shi]

proper watch implementation

I'm guessing this is out-of-scope for Valtio, but perhaps we could find a way to update watch to work as expected for tracking this type of reactivity as #149 originally intended?

I'm very happy to change watch as long as we don't need to touch the Valtio core.

To be honest, I don't think I fully understand "tracking this type of reactivity". watch is originally developed to cover #149, but how is it limited? It would be nice if you can help me understand the spec. Ah, is it just about avoiding get()?

[dai-shi]

Found this:

valtio/docs/api/utils/watch.mdx

Lines 79 to 85 in 2587214

	## No usage tracking
	`watch` currently does not implement the usage tracking of `useSnapshot`, so the callback will rerun anytime a watched proxy (or child proxy) has any fields mutated, regardless of the fields accessed within the `callback`'s code.
	And the return value of `get` is just the proxy itself, not a snapshot.
	This is because `watch` is a vanilla primitive built on top of `subscribe`. It is potentially possible for `watch`, or a new `watch`-like method, to implement usage tracking in the future, see [this discussion](https://github.com/pmndrs/valtio/discussions/640) if interested.

and #640.

[dai-shi]

https://github.com/valtiojs/valtio-reactive implements watch without get. I will deprecate watch in valtio/utils.

[dai-shi]

computed cache

another feature to consider would be supporting caching for computed proxy getters, which I believe would solve this benchmark issue.

I'm actually interested in it. Computeds are one of most requested features from the start. We had derive implementation and usage with proxy-memoize. It's moved to https://github.com/valtiojs/derive-valtio. We are not actively working on this direction as it's pretty complicated, especially for glitch-free. But, I still like the idea of doing such things outside the core.

So, in Valtio, our recommended pattern for computeds is now using object getters. This it not actually a computed. But, it covers lots of Valtio use cases with React. It's because the caching works with snapshot.

So, would you be able to try something like this?

const p = proxy({
  count: 0,
  get double() {
    // some heavy computation here
    return this.count * 2;
  },
});

const s1 = snapshot(p);
const s2 = snapshot(p);
s1 === s2 // is true as snapshots are cached

Is it possible to use this behavior in your benchmark?

[dai-shi]

In https://github.com/valtiojs/valtio-reactive, computeds are cached.

[dai-shi]

possible core extension

I'm sure you want to keep the Valtio project core itself minimal and true to its goals.

That is very true.

The only reason I think it's worth discussing is that "Valtio makes proxy-state simple for React and Vanilla" seems to align pretty well with this sort of direction. 👀 😄

Having that said, I'm actually open to suggestions.
It's because of this: https://x.com/dai_shi/status/1824599433662828682

I'd be curious if you have any ideas on how Valtio could be extended in this directly or if you've seen any other projects which build off of Valtio/ImmutableJS/Immer to add this type of Signals-like reactivity based off of deep, Proxy-based tracking?

Can you help me understand the goal here? Is it like tracking based on state.some.thing access?

#931 exposes some internals, and we can experiment various things outside the core.
(I also consider supporting "batch" if we go this direction.)

[dai-shi]

https://github.com/valtiojs/valtio-reactive uses the Valtio internals and provides batch.