e4cc4a61ff
* Add a jest project for heap retention specs Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012b67ubbXJVMuQrpuEksbeh * Scope the parsed x value cache to one getXValues pass Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012b67ubbXJVMuQrpuEksbeh * Assert heap retention on the settled pass, not the first touch Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012b67ubbXJVMuQrpuEksbeh * Key the log timestamp cache on the log object Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012b67ubbXJVMuQrpuEksbeh * Key the schema viewer flow graph on the response object The memo key serialized every table and field of the schema, so each distinct schema held a megabyte-scale string forever. RTK Query returns a stable reference per cache entry, so a WeakMap keeps the identity that useGraphSync depends on without retaining the schema. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012b67ubbXJVMuQrpuEksbeh * Key pie.rows on the dataset instead of a JSON copy of it The memo key serialized every row and column, so each dataset the user viewed held roughly its own size in cache key for the life of the tab. A WeakMap on the dataset with a short settings subkey keeps the same hit rate. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012b67ubbXJVMuQrpuEksbeh * Ban new uses of underscore's memoize The 17 remaining call sites are allowlisted. The list may only shrink. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012b67ubbXJVMuQrpuEksbeh * Narrow the memoize rule to module scope A cache built inside a function, a component render, a class field or a constructor is released with whatever owns it. Only a module scope cache lives for the life of the tab, so the rule now flags just those. This drops the allowlist from 17 files to 5, and each remaining entry carries the reason its set of keys stays small. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012b67ubbXJVMuQrpuEksbeh * Remove the memoize allowlist Three of the five module scope caches are gone: - getScrollBarSize takes no arguments, so it is a lazy value, not a cache. - getScheduleExplanation returns a string that no caller compares by identity. - cronToBuilderValue moves to the one caller that needs a stable identity. Schedule takes its value as a useMemo dependency, so StrategyForm now holds that memo itself. A user can write any number of cron strings, so a module cache for them was unbounded. The remaining two carry an eslint-disable with the reason their set of keys stays small. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012b67ubbXJVMuQrpuEksbeh * Follow renderer_utils into the viz-core module Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012b67ubbXJVMuQrpuEksbeh * Cover metabase/utils/memoize in the module scope rule That memoize roots its WeakMap on the function it wraps, so it only releases once that function does. At module scope the function never does, and the nested Maps hold every argument ever passed. No call site does this today, but the rule could not have caught one. The rule now covers both sources, so it drops "underscore" from its name. memoizeClass stays allowed: it keys on the instance. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Drop re-reselect from the dashboard selectors createCachedSelector keyed getQuestionByCard and getDashcardParameterMappingOptions on the card id, a primitive, in a FlatObjectCache. That cache is a plain object with no eviction, so a session kept one Question per card id it ever saw, and each Question pins the whole Metadata snapshot it was built with. reselect v5 defaults to weakMapMemoize, which keys on the card and the metadata objects themselves. Entries are released when their card leaves the store or the metadata is replaced, and identity still holds for every card in the working set, which is what connect and useSelector compare on. Measured over 300 cards, with the states dropped: 6.2 MB retained before, 0.0 MB after. Per entry, 21462 B before and 51 B after. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Drop re-reselect from the lockfile Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Back metabase/utils/memoize with reselect's weakMapMemoize The hand-rolled implementation had two problems. It allocated two arrays on every call, including cache hits: 225 ms against 20 ms for weakMapMemoize over two million hits. And its WeakMap was keyed on the wrapped function rather than the wrapper, so a cache built around a module-level helper outlived the wrapper that owned it. A plain Map would have been safer. weakMapMemoize matches on the same terms, identity for objects and value for primitives, so no call site changes. It also weak-keys object arguments, which the old one held strongly. definition-cache passed weakMapMemoize as the memoize and argsMemoize options. reselect v5 already defaults to both, so those are dropped. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Route all memoization through metabase/utils/memoize underscore's memoize is gone from the app. Its 16 call sites now use the house helper, which is reselect's weakMapMemoize. Three of them were in metabase-lib, which cannot import metabase/utils, and all three took no arguments, so they became plain lazy fields instead. Two rules keep it that way: - no-direct-memoize-import bans memoize from underscore and weakMapMemoize or lruMemoize from reselect, in any scope. Only metabase/utils/memoize may reach for an implementation. - no-module-level-memoize keeps its job, now that the house helper is the only one left to police. measureTextWidthSafely took a style object that was rebuilt on every render, so identity keying would never have hit. Its third argument is now the font family string, which is the only field any caller ever set. useMemoizedCallback returns the callable shape rather than casting to T, since the memoized function also carries clearCache and resultsCount. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Build memoizeClass on the house memoize It kept a module-level WeakMap and walked a nested Map trie keyed on the instance, the method, the argument count and then each argument, assembling that path into an array on every call. Keying weakMapMemoize on the instance and then the arguments gives the same per-instance caching. One cache per decorated method replaces the shared trie, so the method no longer needs to be part of the key, and neither does the argument count. The file is down to 61 lines from 128. A leak spec covers the property this rests on: an instance's results are released with the instance. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Measure memoizeClass retention under load 2,000 instances per pass, 20 calls each, across a method with no arguments and one taking a primitive and an object. Nothing is retained once the instances are dropped. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Delete memoizeClass It decorated a prototype method by rewriting the property descriptor, and put the memoization 600 lines away from the method it applied to. Its one consumer memoized a method whose body uses no `this`, so the same caching comes from assigning a memoized arrow to a class field. The field is created per instance, so the cache is released with the instance, and the memoization is now visible at the definition. metabase/utils/memoize is 14 lines, from 128. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Run the heap specs from their own jest config They were a project inside jest.config.js, and a bare `jest` runs every project, which is what CI does. So the sharded unit run picked them up, without --expose-gc, and they failed as designed. They now live in jest.memory.conf.js, the way the timezone specs live in their own config. The shared base moves to jest.base.conf.js so neither file has to duplicate it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Reduce the heap specs to one collection assertion each Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012b67ubbXJVMuQrpuEksbeh * Follow the store mocks into test support Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012b67ubbXJVMuQrpuEksbeh --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
25 lines
638 B
JavaScript
25 lines
638 B
JavaScript
// @ts-check
|
|
|
|
/**
|
|
* Heap retention specs.
|
|
*
|
|
* They need --expose-gc and a heap nobody else is touching, so they cannot join
|
|
* the sharded default run. Use `bun run test-memory`.
|
|
*/
|
|
|
|
// eslint-disable-next-line import/no-commonjs
|
|
const baseConfig = require("./jest.base.conf.js");
|
|
|
|
/** @type {import('jest').Config} */
|
|
// eslint-disable-next-line import/no-commonjs
|
|
module.exports = {
|
|
...baseConfig,
|
|
displayName: "memory",
|
|
setupFilesAfterEnv: [
|
|
...baseConfig.setupFilesAfterEnv,
|
|
"<rootDir>/frontend/test/jest-setup-env-core.js",
|
|
],
|
|
testMatch: ["<rootDir>/**/*.leak.unit.spec.{ts,tsx}"],
|
|
reporters: ["default"],
|
|
};
|