From 571d2c0d916da43bf507a9a0bc336021bd63df44 Mon Sep 17 00:00:00 2001 From: littensy <56808540+littensy@users.noreply.github.com> Date: Thu, 8 May 2025 00:04:00 -0700 Subject: [PATCH] refactor: sync 2.0.4 --- benchmarks/mapped.luau | 7 +-- pkgs/charm-sync/tests/spec.luau | 12 ++-- pkgs/charm/src/listen.luau | 7 +-- pkgs/charm/src/mapped.luau | 9 +-- pkgs/charm/src/observe.luau | 7 +-- pkgs/charm/src/signals.luau | 43 +++++++------- pkgs/charm/src/subscribe.luau | 7 +-- pkgs/charm/src/untracked.luau | 7 +-- pkgs/charm/tests/signals/effect.spec.luau | 19 +++--- pkgs/charm/tests/signals/gc.spec.luau | 72 +++++++++++++++++++++++ pkgs/charm/tests/signals/spec.luau | 15 +++-- pkgs/charm/tests/spec.luau | 9 +-- pkgs/react-charm/tests/spec.luau | 4 +- pkgs/vide-charm/tests/spec.luau | 4 +- tests/spec.server.luau | 15 +---- 15 files changed, 152 insertions(+), 85 deletions(-) create mode 100644 pkgs/charm/tests/signals/gc.spec.luau diff --git a/benchmarks/mapped.luau b/benchmarks/mapped.luau index 327197d..05358c1 100644 --- a/benchmarks/mapped.luau +++ b/benchmarks/mapped.luau @@ -4,8 +4,7 @@ local bench = require("./bench") local atom = Charm.atom local computed = Charm.computed local mapped = Charm.mapped -local pauseTracking = Charm.signals.pauseTracking -local resumeTracking = Charm.signals.resumeTracking +local setCurrentSub = Charm.signals.setCurrentSub type Entry = { number } @@ -26,9 +25,9 @@ local function simpleMapped(source: () -> { Entry }, mapper: (Entry, number) -> local nextState = {} for index, value in state do - pauseTracking() + local prevSub = setCurrentSub(nil) local success, valueOut, keyOut = pcall(mapper, value, index) - resumeTracking() + setCurrentSub(prevSub) if success then nextState[keyOut] = valueOut diff --git a/pkgs/charm-sync/tests/spec.luau b/pkgs/charm-sync/tests/spec.luau index b386f15..39ee48c 100644 --- a/pkgs/charm-sync/tests/spec.luau +++ b/pkgs/charm-sync/tests/spec.luau @@ -10,10 +10,12 @@ local function spec(name: string) end) end -spec("client") -spec("server") -spec("flatten") -spec("patch") -spec("validate") +describe("charm-sync", function() + spec("client") + spec("server") + spec("flatten") + spec("patch") + spec("validate") +end) return {} diff --git a/pkgs/charm/src/listen.luau b/pkgs/charm/src/listen.luau index d6c9a31..00c2141 100644 --- a/pkgs/charm/src/listen.luau +++ b/pkgs/charm/src/listen.luau @@ -1,8 +1,7 @@ local signals = require("./signals") local effect = signals.effect -local pauseTracking = signals.pauseTracking -local resumeTracking = signals.resumeTracking +local setCurrentSub = signals.setCurrentSub local function listen(selector: () -> T, listener: (state: T, previous: T?, dispose: () -> ()) -> ()): () -> () local disposeImmediately = false @@ -17,9 +16,9 @@ local function listen(selector: () -> T, listener: (state: T, previous: T?, d current = selector() if current ~= previous then - pauseTracking() + local prevSub = setCurrentSub(nil) local success, result: unknown = pcall(listener, current, previous, dispose) - resumeTracking() + setCurrentSub(prevSub) if not success then error(`listen() encountered an error: {result}`) diff --git a/pkgs/charm/src/mapped.luau b/pkgs/charm/src/mapped.luau index 2c20797..ae4dcff 100644 --- a/pkgs/charm/src/mapped.luau +++ b/pkgs/charm/src/mapped.luau @@ -1,10 +1,7 @@ ---!optimize 2 - local signals = require("./signals") local computed = signals.computed -local pauseTracking = signals.pauseTracking -local resumeTracking = signals.resumeTracking +local setCurrentSub = signals.setCurrentSub type Mapped = & ((selector: () -> { [KI]: VI }, mapper: (item: VI, key: KI) -> (VO, KO)) -> () -> { [KO]: VO }) @@ -19,9 +16,9 @@ local function mapped(selector: () -> { [KI]: VI }, mapper: (ite local keyCache: { [KI]: KO } = {} local function map(value: VI, key: KI): (VO, KO) - pauseTracking() + local prevSub = setCurrentSub(nil) local success, valueOut, keyOut = pcall(mapper, value, key) - resumeTracking() + setCurrentSub(prevSub) if not success then error(`mapped() transform function failed: {valueOut}`) diff --git a/pkgs/charm/src/observe.luau b/pkgs/charm/src/observe.luau index 8cce9cb..a64c9b3 100644 --- a/pkgs/charm/src/observe.luau +++ b/pkgs/charm/src/observe.luau @@ -3,10 +3,9 @@ local signals = require("./signals") local effect = signals.effect local effectScope = signals.effectScope local getCurrentSub = signals.getCurrentSub +local setCurrentSub = signals.setCurrentSub local onEffectCleanup = signals.onEffectCleanup local onScopeDispose = signals.onScopeDispose -local pauseTracking = signals.pauseTracking -local resumeTracking = signals.resumeTracking type Cleanup = () -> () @@ -47,9 +46,9 @@ local function observe(selector: () -> { [K]: V }, observer: (item: V, key currentItems = selector() if currentItems ~= previousItems then - pauseTracking() + local prevSub = setCurrentSub(nil) local success, result: unknown = pcall(updateScopes, currentItems, previousItems) - resumeTracking() + setCurrentSub(prevSub) if not success then error(`observe() encountered an error: {result}`, 2) diff --git a/pkgs/charm/src/signals.luau b/pkgs/charm/src/signals.luau index 2cb796b..bb27e45 100644 --- a/pkgs/charm/src/signals.luau +++ b/pkgs/charm/src/signals.luau @@ -62,8 +62,7 @@ type Signal = ReactiveNode & { } local pauseStack: { ReactiveNode? } = {} -local pauseStackCount = 0 -local queuedEffects: { Effect | EffectScope } = {} +local queuedEffects: { Effect | EffectScope? } = {} local link: (dep: ReactiveNode, sub: ReactiveNode) -> () local unlink: (link: Link, sub: ReactiveNode?) -> Link? @@ -193,16 +192,16 @@ local function endBatch() end end +--- @deprecated Will be removed in the next major version. Use +--- `const pausedSub = setCurrentSub(undefined)` instead for better performance. local function pauseTracking() - pauseStackCount += 1 - pauseStack[pauseStackCount] = activeSub - activeSub = nil + table.insert(pauseStack, setCurrentSub(nil)) end +--- @deprecated Will be removed in the next major version. Use +--- `setCurrentSub(pausedSub)` instead for better performance. local function resumeTracking() - activeSub = pauseStack[pauseStackCount] - pauseStack[pauseStackCount] = nil - pauseStackCount -= 1 + setCurrentSub(table.remove(pauseStack)) end local function updateComputed(c: Computed): boolean @@ -311,9 +310,8 @@ local function effectStop(this: Effect | EffectScope): () -> () dep = unlink(dep, this) end local sub = this.subs - while sub do + if sub then unlink(sub) - sub = this.subs end (this :: Effect).flags = NONE cleanupEffect(this) @@ -361,9 +359,11 @@ local function effectScope(fn: () -> (), detached: boolean?): () -> () if activeScope and not detached then link(e, activeScope) end - local prev = setCurrentScope(e) + local prevSub = setCurrentSub(nil) + local prevScope = setCurrentScope(e) local success, result: unknown = pcall(fn) - setCurrentScope(prev) + setCurrentScope(prevScope) + setCurrentSub(prevSub) if not success then error(result) end @@ -392,15 +392,18 @@ local function notify(e: Effect | EffectScope) end end -local function unwatched(signal: Signal | Effect | Computed) - local toRemove = signal.deps - if toRemove then - repeat - toRemove = unlink(toRemove, signal) - until toRemove == nil - (signal :: Signal).flags = bit32.bor(signal.flags, DIRTY) +local function unwatched(node: Signal | Computed | Effect | EffectScope) + if (node :: Computed).getter then + local toRemove = node.deps + if toRemove then + (node :: Computed).flags = bit32.bor(MUTABLE, DIRTY) + repeat + toRemove = unlink(toRemove, node) + until not toRemove + end + elseif (node :: Signal).previousValue == nil then + effectStop(node :: Effect)() end - cleanupEffect(signal :: Effect) end local reactiveSystem = createReactiveSystem({ diff --git a/pkgs/charm/src/subscribe.luau b/pkgs/charm/src/subscribe.luau index b038a4a..48a2300 100644 --- a/pkgs/charm/src/subscribe.luau +++ b/pkgs/charm/src/subscribe.luau @@ -1,8 +1,7 @@ local signals = require("./signals") local effect = signals.effect -local pauseTracking = signals.pauseTracking -local resumeTracking = signals.resumeTracking +local setCurrentSub = signals.setCurrentSub local function subscribe(selector: () -> T, listener: (state: T, lastState: T) -> ()): () -> () local current: T @@ -15,9 +14,9 @@ local function subscribe(selector: () -> T, listener: (state: T, lastState: T if mounted then mounted = false elseif current ~= previous then - pauseTracking() + local prevSub = setCurrentSub(nil) local success, result: unknown = pcall(listener, current, previous) - resumeTracking() + setCurrentSub(prevSub) if not success then error(`subscribe() encountered an error: {result}`) diff --git a/pkgs/charm/src/untracked.luau b/pkgs/charm/src/untracked.luau index d494345..d897e6b 100644 --- a/pkgs/charm/src/untracked.luau +++ b/pkgs/charm/src/untracked.luau @@ -1,12 +1,11 @@ local signals = require("./signals") -local pauseTracking = signals.pauseTracking -local resumeTracking = signals.resumeTracking +local setCurrentSub = signals.setCurrentSub local function untracked(callback: ((Args...) -> T) | (Args...) -> (), ...: Args...): T - pauseTracking() + local prevSub = setCurrentSub(nil) local success, result = pcall(callback, ...) - resumeTracking() + setCurrentSub(prevSub) if not success then error(result) diff --git a/pkgs/charm/tests/signals/effect.spec.luau b/pkgs/charm/tests/signals/effect.spec.luau index 7fb2546..76e2fda 100644 --- a/pkgs/charm/tests/signals/effect.spec.luau +++ b/pkgs/charm/tests/signals/effect.spec.luau @@ -303,9 +303,14 @@ end) test("should run inner effect cleanup", function() local cleanups = 0 - local a, setA = signal(false) + local src, setSrc = signal(false) local stopEffect = effect(function() - a() + src() + effect(function() + onEffectCleanup(function() + cleanups += 1 + end) + end) effect(function() onEffectCleanup(function() cleanups += 1 @@ -314,12 +319,12 @@ test("should run inner effect cleanup", function() end) assert(cleanups == 0, "should not cleanup immediately") - setA(true) - assert(cleanups == 1, "should cleanup after outer update") + setSrc(true) + assert(cleanups == 2, "should cleanup after outer update") stopEffect() - assert(cleanups == 2, "should cleanup after outer dispose") - setA(false) - assert(cleanups == 2, "should not cleanup after update") + assert(cleanups == 4, "should cleanup after outer dispose") + setSrc(false) + assert(cleanups == 4, "should not cleanup after update") end) return {} diff --git a/pkgs/charm/tests/signals/gc.spec.luau b/pkgs/charm/tests/signals/gc.spec.luau new file mode 100644 index 0000000..e08d265 --- /dev/null +++ b/pkgs/charm/tests/signals/gc.spec.luau @@ -0,0 +1,72 @@ +local Charm = require("@pkgs/charm/src/lib") +local suite = require("@tests/suite") + +local atom = Charm.atom +local computed = Charm.computed +local effect = Charm.effect +local effectScope = Charm.effectScope +local test = suite.test + +--- Creates a weak reference to a value. +--- @return Function to dereference the value. +local function weakRef(value: T): () -> T? + local ref = setmetatable({ value = value }, { __mode = "v" }) + return function() + return ref.value + end +end + +--- Triggers garbage collection by allocating excessive memory. +local function gc() + for _ = 1, 1000 do + local _ = table.create(100, 0) + end +end + +--- In alien-signals, a computed value unlinks from dependencies when its last +--- parent scope is disposed. Calling a computed value outside of a scope links +--- it to dependencies, but the dependencies hold strong references to the +--- computed value, so it won't GC until called in a scope and disposed. +--- +--- This function releases a computed value by linking it to a scope and then +--- immediately closing it, causing alien-signals to unlink the computed value +--- and allow it to be garbage collected. +local function release(getter: () -> ()) + effectScope(function() + getter() + end)() +end + +test("should release computed cache", function() + local srcRef = weakRef({}) + local src = atom(srcRef()) + + local c = computed(function() + return src() + end) + + c() -- cache src value + src(nil) -- release value + c = release(c) -- release computed + + gc() + assert(srcRef() == nil, "should release computed cache") +end) + +test("should not release effect for atom", function() + local effectCalls = 0 + local src = atom(0) + + effect(function() + effectCalls += 1 + src() + end) + + assert(effectCalls == 1, "should call effect once") + + gc() + src(src() + 1) + assert(effectCalls == 2, "should call effect again") +end) + +return {} diff --git a/pkgs/charm/tests/signals/spec.luau b/pkgs/charm/tests/signals/spec.luau index fcc4a57..15c69f5 100644 --- a/pkgs/charm/tests/signals/spec.luau +++ b/pkgs/charm/tests/signals/spec.luau @@ -8,11 +8,14 @@ local function spec(name: string) end) end -spec("computed") -spec("effect") -spec("effectScope") -spec("issue_48") -spec("topology") -spec("untrack") +describe("signals", function() + spec("computed") + spec("effect") + spec("effectScope") + spec("issue_48") + spec("gc") + spec("topology") + spec("untrack") +end) return {} diff --git a/pkgs/charm/tests/spec.luau b/pkgs/charm/tests/spec.luau index fc8f473..f86a562 100644 --- a/pkgs/charm/tests/spec.luau +++ b/pkgs/charm/tests/spec.luau @@ -2,19 +2,14 @@ local suite = require("@tests/suite") local describe = suite.describe -local function collect(name: string) - describe(name, function() - return require(`./{name}/spec`) :: any - end) -end - local function spec(name: string) describe(name, function() return require(`./{name}.spec`) :: any end) end -collect("signals") +require("./signals/spec") + spec("gc") spec("listen") spec("mapped") diff --git a/pkgs/react-charm/tests/spec.luau b/pkgs/react-charm/tests/spec.luau index 6cd8895..2ce9682 100644 --- a/pkgs/react-charm/tests/spec.luau +++ b/pkgs/react-charm/tests/spec.luau @@ -10,6 +10,8 @@ local function spec(name: string) end) end -spec("useAtom") +describe("react-charm", function() + spec("useAtom") +end) return {} diff --git a/pkgs/vide-charm/tests/spec.luau b/pkgs/vide-charm/tests/spec.luau index 6cd8895..2bde270 100644 --- a/pkgs/vide-charm/tests/spec.luau +++ b/pkgs/vide-charm/tests/spec.luau @@ -10,6 +10,8 @@ local function spec(name: string) end) end -spec("useAtom") +describe("vide-charm", function() + spec("useAtom") +end) return {} diff --git a/tests/spec.server.luau b/tests/spec.server.luau index da42dc8..a3d8b9b 100644 --- a/tests/spec.server.luau +++ b/tests/spec.server.luau @@ -1,6 +1,5 @@ local suite = require("./suite") -local describe = suite.describe local finish = suite.finish if not script then @@ -9,16 +8,8 @@ end _G.__DEV__ = true -describe("charm-sync", function() - require(script.Parent.CharmSync.spec) -end) - -describe("react-charm", function() - require(script.Parent.ReactCharm.spec) -end) - -describe("vide-charm", function() - require(script.Parent.VideCharm.spec) -end) +require(script.Parent.CharmSync.spec) +require(script.Parent.ReactCharm.spec) +require(script.Parent.VideCharm.spec) finish()