Skip to content

Commit

Permalink
Avoid using a nested WeakMap for manager instances for a given owner.
Browse files Browse the repository at this point in the history
The first level WeakMap is sufficient to prevent memory leaks. The only
potential value that the second level WeakMap provides is to allow us to
_unload_ code during the lifetime of a single owner instance, which is
really not something that we support.

Having nested WeakMap's breaks some tooling around memory leak
investigation that we commonly use.
  • Loading branch information
rwjblue committed Oct 18, 2021
1 parent 0542f62 commit 30d57fc
Showing 1 changed file with 2 additions and 5 deletions.
7 changes: 2 additions & 5 deletions packages/@glimmer/runtime/lib/managers/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,10 +40,7 @@ const HELPER_MANAGERS = new WeakMap<
ManagerFactory<Owner | undefined, HelperManager<unknown> | Helper>
>();

const OWNER_MANAGER_INSTANCES: WeakMap<
Owner,
WeakMap<ManagerFactory<Owner>, unknown>
> = new WeakMap();
const OWNER_MANAGER_INSTANCES: WeakMap<Owner, Map<ManagerFactory<Owner>, unknown>> = new WeakMap();
const UNDEFINED_MANAGER_INSTANCES: WeakMap<ManagerFactory<Owner>, unknown> = new WeakMap();

export type ManagerFactory<O, D extends ManagerDelegate = ManagerDelegate> = (owner: O) => D;
Expand Down Expand Up @@ -107,7 +104,7 @@ function getManagerInstanceForOwner<D extends ManagerDelegate>(
managers = OWNER_MANAGER_INSTANCES.get(owner);

if (managers === undefined) {
managers = new WeakMap();
managers = new Map();
OWNER_MANAGER_INSTANCES.set(owner, managers);
}
}
Expand Down

0 comments on commit 30d57fc

Please sign in to comment.