Shrink ComputedValue using a bitfield

[peterm-canva]

Code change checklist

• [] N/A: Added/updated unit tests
• [] N/A: Updated /docs. For new functionality, at least API.md should be updated
• [x] Verified that there is no significant performance drop (yarn mobx test:performance)

ComputedValue is currently 112 bytes in 64-bit chromium browsers, and it's not unreasonable to see 10s or 100s of thousands of these on very large apps. This makes this class IMO a reasonable target for memory optimization, given small changes can give big wins in absolute memory.

This change takes four boolean fields and stores them in one bitfield, adding getters and setters to keep the API to access them the same.

This shrinks 112 bytes down to 100 (verified in devtools) by removing 4x 4 byte booleans and replacing them with 1x 4 byte integer in v8. In Safari the bools take 8 bytes so this should save 3x 8 bytes (but I didn't check).

There will be an increased overhead to get/set the boolean values now given the bit-shifting required. I don't see any regressions running yarn test:performance, but I'm happy to investigate further if this is a concern. I don't think these booleans are in any really performance critical paths. If they are, I'm pretty confident v8 at least will optimize the bit-shifting well, and only add a few machine instructions in optimized code.

Regarding the code, using a bitfield could be done in many different ways, so take what I have here as one suggestion, I'm happy to refactor or make more general. I'd like to avoid introducing a bitfield class, because the allocation of a new object per ComputedValue instance will cancel out the memory savings. Better to keep the bitfield as a direct member of ComputedValue.

If we can reach a suitable solution here, I'd like to do the same thing to ObservableValue, Atom, Reaction and ObservableObjectAdministration which from a brief glance can all save 12-ish bytes as well.

[changeset-bot[bot]]

🦋 Changeset detected

Latest commit: ff1d3dd0d7f05ba3cdea9ff3a58862648fac8285

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
mobx	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[peterm-canva]

Not that familiar with changesets but this is an implementation detail so I think doesn't need a changeset. Otherwise I'll add one.

[peterm-canva]

@urugator are you able to help with review? Many thanks

[peterm-canva]

Added a patchset because @taj-p mentioned I will most likely need that so we can uprev to that version.

[mweststrate]

Thanks for this PR, this looks great!

If we can reach a suitable solution here, I'd like to do the same thing to ObservableValue, Atom, Reaction and ObservableObjectAdministration which from a brief glance can all save 12-ish bytes as well.

I imagine it'd be good to have the release soak for a few weeks, if it doesn't raise any issues sounds good :)

[peterm-canva]

I imagine it'd be good to have the release soak for a few weeks, if it doesn't raise any issues sounds good :)

Sounds good, I'll get the next PR ready and we can hold off a few weeks.

[peterm-canva]

There's an issue with this for the production build. The getters/setters are not renamed by minification, but the code that accesses them does get the keys renamed. This means they always come back falsey. Working on that

[peterm-canva]

OK, good to go. I had to change names so they aren't mangled any more. See comment in atom.ts.

[peterm-canva]

Can we land this one?

[mweststrate]

Sorry, missed the update! Merged!

[mweststrate]

Should soon be available as 6.12.4

@peterm-canva since you are validating performance, I am wondering if you are interested in checking if https://github.com/mobxjs/mobx/pull/3884 also gains these positive results for you?