Skip to content

Commit

Permalink
fix: (perf) Remove extra clone operation on .values .keys .Iterator a…
Browse files Browse the repository at this point in the history
…ccess (#3993)

* Perf(observableset): remove unwanted clone from `entries`, `values` methods

* Add performance test

* Add changeset

* Omit redundant keys variable

* More verbose perf test logs
  • Loading branch information
tonyraoul authored Jan 30, 2025
1 parent f5e9472 commit bca3841
Show file tree
Hide file tree
Showing 3 changed files with 156 additions and 18 deletions.
5 changes: 5 additions & 0 deletions .changeset/odd-dogs-thank.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"mobx": patch
---

Improve observableset memory footprint and performance
148 changes: 143 additions & 5 deletions packages/mobx/__tests__/perf/perf.js
Original file line number Diff line number Diff line change
Expand Up @@ -203,11 +203,11 @@ results of this test:
const ar = observable([0])
const findLastIndexOfZero = computed(function () {
aCalc++
return ar.findLastIndex(x => x === 0);
return ar.findLastIndex(x => x === 0)
})
const lastIndexOfZero = computed(function () {
bCalc++
return ar.lastIndexOf(0);
return ar.lastIndexOf(0)
})
mobx.observe(findLastIndexOfZero, voidObserver, true)
mobx.observe(lastIndexOfZero, voidObserver, true)
Expand All @@ -225,9 +225,7 @@ results of this test:

const end = now()

log(
"Array findLastIndex loop - Updated in " + (end - start) + " ms."
)
log("Array findLastIndex loop - Updated in " + (end - start) + " ms.")
t.end()
})

Expand Down Expand Up @@ -552,6 +550,146 @@ results of this test:
t.end()
})

test(`${version} - Set: initializing`, function (t) {
gc()
const iterationsCount = 100000
let i

const start = Date.now()
for (i = 0; i < iterationsCount; i++) {
mobx.observable.set()
}
const end = Date.now()
log("Initilizing " + iterationsCount + " maps: " + (end - start) + " ms.")
t.end()
})

test(`${version} - Set: setting and deleting properties`, function (t) {
gc()
const iterationsCount = 1000
const propertiesCount = 10000
const set = mobx.observable.set()
let i
let p

const start = Date.now()
for (i = 0; i < iterationsCount; i++) {
for (p = 0; p < propertiesCount; p++) {
set.add("" + p)
}
for (p = 0; p < propertiesCount; p++) {
set.delete("" + p)
}
}
const end = Date.now()

log(
"Setting and deleting " +
propertiesCount +
" set properties " +
iterationsCount +
" times: " +
(end - start) +
" ms."
)
t.end()
})

test(`${version} - Set: looking up properties`, function (t) {
gc()
const iterationsCount = 1000
const propertiesCount = 10000
const set = mobx.observable.set()
let i
let p

for (p = 0; p < propertiesCount; p++) {
set.add("" + p)
}

const start = Date.now()
for (i = 0; i < iterationsCount; i++) {
for (p = 0; p < propertiesCount; p++) {
set.has("" + p)
}
}
const end = Date.now()

log(
"Looking up " +
propertiesCount +
" set properties " +
iterationsCount +
" times: " +
(end - start) +
" ms."
)
t.end()
})

test(`${version} - Set: iterator helpers`, function (t) {
gc()
const iterationsCount = 1000
const propertiesCount = 10000
const set = mobx.observable.set()
let i
let p

for (p = 0; p < propertiesCount; p++) {
set.add("" + p)
}

const start = Date.now()
for (i = 0; i < iterationsCount; i++) {
set.entries().take(1)
}
const end = Date.now()

log(
"Single take out of" +
propertiesCount +
" set properties " +
iterationsCount +
" times: " +
(end - start) +
" ms."
)
t.end()
})

test(`${version} - Set: conversion to array`, function (t) {
gc()
const iterationsCount = 1000
const propertiesCount = 10000
const set = mobx.observable.set()
let i
let p

for (p = 0; p < propertiesCount; p++) {
set.add("" + p)
}

const start = Date.now()
for (i = 0; i < iterationsCount; i++) {
Array.from(set.keys())
Array.from(set.values())
Array.from(set.entries())
;[...set]
}
const end = Date.now()

log(
"Converting " +
propertiesCount +
" set properties into an array" +
iterationsCount +
" times: " +
(end - start) +
" ms."
)
t.end()
})

test(`${version} - Map: initializing`, function (t) {
gc()
const iterationsCount = 100000
Expand Down
21 changes: 8 additions & 13 deletions packages/mobx/src/types/observableset.ts
Original file line number Diff line number Diff line change
Expand Up @@ -211,16 +211,11 @@ export class ObservableSet<T = any> implements Set<T>, IInterceptable<ISetWillCh
}

entries() {
let nextIndex = 0
const keys = Array.from(this.keys())
const values = Array.from(this.values())
const values = this.values()
return makeIterableForSet<[T, T]>({
next() {
const index = nextIndex
nextIndex += 1
return index < values.length
? { value: [keys[index], values[index]], done: false }
: { value: undefined, done: true }
const { value, done } = values.next()
return !done ? { value: [value, value], done } : { value: undefined, done }
}
})
}
Expand All @@ -232,13 +227,13 @@ export class ObservableSet<T = any> implements Set<T>, IInterceptable<ISetWillCh
values(): SetIterator<T> {
this.atom_.reportObserved()
const self = this
let nextIndex = 0
const observableValues = Array.from(this.data_.values())
const values = this.data_.values()
return makeIterableForSet({
next() {
return nextIndex < observableValues.length
? { value: self.dehanceValue_(observableValues[nextIndex++]), done: false }
: { value: undefined, done: true }
const { value, done } = values.next()
return !done
? { value: self.dehanceValue_(value), done }
: { value: undefined, done }
}
})
}
Expand Down

0 comments on commit bca3841

Please sign in to comment.