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

Ensure ComponentDefinitions do not leak heap space. #16558

Merged
merged 3 commits into from
Apr 21, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 44 additions & 6 deletions packages/ember-glimmer/lib/resolver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,10 @@ import {
} from '@glimmer/interfaces';
import { LazyCompiler, Macros, PartialDefinition } from '@glimmer/opcode-compiler';
import { ComponentManager, getDynamicVar, Helper, ModifierManager } from '@glimmer/runtime';
import { privatize as P } from 'container';
import { FACTORY_FOR, privatize as P } from 'container';
import { ENV } from 'ember-environment';
import { LookupOptions, Owner, setOwner } from 'ember-owner';
import { guidFor } from 'ember-utils';
import { lookupComponent, lookupPartial, OwnedTemplateMeta } from 'ember-views';
import CompileTimeLookup from './compile-time-lookup';
import { CurlyComponentDefinition } from './component-managers/curly';
Expand Down Expand Up @@ -55,6 +56,15 @@ function makeOptions(moduleName: string, namespace?: string): LookupOptions {
};
}

// returns the qualified / expanded name
// which accounts for local lookup...
function getNormalizedName(obj: any) {
let factoryManager = FACTORY_FOR.get(obj);
if (factoryManager) {
return factoryManager.normalizedName;
}
}

const BUILTINS_HELPERS = {
if: inlineIf,
action,
Expand Down Expand Up @@ -100,9 +110,12 @@ export default class RuntimeResolver implements IRuntimeResolver<OwnedTemplateMe

// supports directly imported late bound layouts on component.prototype.layout
private templateCache: WeakMap<Owner, WeakMap<TemplateFactory, OwnedTemplate>> = new WeakMap();
private componentDefinitionCache: Map<string, ComponentDefinition | null> = new Map();

public templateCacheHits = 0;
public templateCacheMisses = 0;
public componentDefinitionCount = 0;
public helperDefinitionCount = 0;

constructor() {
let macros = new Macros();
Expand All @@ -113,6 +126,7 @@ export default class RuntimeResolver implements IRuntimeResolver<OwnedTemplateMe
/*** IRuntimeResolver ***/

/**
* public componentDefHandleCount = 0;
* Called while executing Append Op.PushDynamicComponentManager if string
*/
lookupComponentDefinition(name: string, meta: OwnedTemplateMeta): Option<ComponentDefinition> {
Expand All @@ -127,7 +141,12 @@ export default class RuntimeResolver implements IRuntimeResolver<OwnedTemplateMe
}

lookupComponentHandle(name: string, meta: OwnedTemplateMeta) {
return this.handle(this._lookupComponentDefinition(name, meta));
let nextHandle = this.handles.length;
let handle = this.handle(this._lookupComponentDefinition(name, meta));
if (nextHandle === handle) {
this.componentDefinitionCount++;
}
return handle;
}

/**
Expand All @@ -142,9 +161,15 @@ export default class RuntimeResolver implements IRuntimeResolver<OwnedTemplateMe
* Called by CompileTimeLookup compiling Unknown or Helper OpCode
*/
lookupHelper(name: string, meta: OwnedTemplateMeta): Option<number> {
let handle = this._lookupHelper(name, meta);
if (handle !== null) {
return this.handle(handle);
let nextHandle = this.handles.length;
let helper = this._lookupHelper(name, meta);
if (helper !== null) {
let handle = this.handle(helper);

if (nextHandle === handle) {
this.helperDefinitionCount++;
}
return handle;
}
return null;
}
Expand Down Expand Up @@ -291,8 +316,18 @@ export default class RuntimeResolver implements IRuntimeResolver<OwnedTemplateMe
makeOptions(meta.moduleName, namespace)
);

let ownerId = guidFor(meta.owner);
let componentDefinitionCacheKey =
ownerId + '|' + getNormalizedName(component) + '|' + getNormalizedName(layout);
let cachedComponentDefinition = this.componentDefinitionCache.get(componentDefinitionCacheKey);
if (cachedComponentDefinition !== undefined) {
return cachedComponentDefinition;
}

if (layout && !component && ENV._TEMPLATE_ONLY_GLIMMER_COMPONENTS) {
return new TemplateOnlyComponentDefinition(layout);
let definition = new TemplateOnlyComponentDefinition(layout);
this.componentDefinitionCache.set(componentDefinitionCacheKey, definition);
return definition;
}

let manager:
Expand All @@ -317,6 +352,9 @@ export default class RuntimeResolver implements IRuntimeResolver<OwnedTemplateMe
: null;

finalizer();

this.componentDefinitionCache.set(componentDefinitionCacheKey, definition);

return definition;
}
}
131 changes: 0 additions & 131 deletions packages/ember-glimmer/tests/unit/layout-cache-test.js

This file was deleted.

Loading