Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Perf(observableset): Remove extra clone operation on read #3993

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

tonyraoul
Copy link
Contributor

@tonyraoul tonyraoul commented Jan 17, 2025

Code change checklist

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

Context

within the observableset, on retriving entries/keys/values or during iteration, there is an extra Array.from operation that is causing an unwanted copy of values within memory, the significance of extra operation is amplified when considering iterator helper methods where it would read the entire range of elements within the set instead of lazy evaluation, i.e: Set.prototype.entries().take(1), or any other case when the iterator is partially retrieved Set.prototype.entries().next().

Benchmarks

Time

Use a macbook air M1 - 8GB model - plugged in, 1000 iterations

test before after
Set: conversion to array - 1000 elements 3.33s min - 3.41s max 2.90s min - 2.95s max
Set: conversion to array - 10000 elements 12.74s min - 12.87s max 9.10s min - 9.16s max
Set: iterator helpers - 1000 elements, take(1) 0.60s min - 0.77s max 0.31s min - 0.32s max
Set: iterator helpers - 10000 elements, take(1) 4.19s min - 4.22s max 0.31s min - 0.32s max

Memory

Mesuring the effect on memory when converting a large observableset to an array.

size before after
1M item 66 Mb 46 Mb
10M item 617 Mb 423 Mb
   function benchmarkMemoryUsage() {
            const beforeMemory = memoryUsage().heapUsed
            
            const obs = observable.set()
            for (let i = 0; i < 10000_000; i++) {
                obs.add(i)
            }

            const x = Array.from(obs)
            const afterMemory1 = memoryUsage().heapUsed

            console.log(`Memory diff: ${(afterMemory1 - beforeMemory) / 1000_000} Mbytes %`)
        }

Copy link

changeset-bot bot commented Jan 17, 2025

🦋 Changeset detected

Latest commit: b4d3074

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

@tonyraoul tonyraoul marked this pull request as ready for review January 17, 2025 13:29
@tonyraoul tonyraoul changed the title Perf(observableset): Remove extra clone operation on read/write Perf(observableset): Remove extra clone operation on read Jan 17, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant