Skip to content

Commit e34146f

Browse files
tboschtbosch
committed
fix(view_factory): fix caching of views
Previous implementation had bugs, and did not cache per ProtoView.
1 parent 4e2316c commit e34146f

6 files changed

Lines changed: 195 additions & 44 deletions

File tree

‎modules/angular2/src/core/application.js‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -96,15 +96,15 @@ function _injectorBindings(appComponentType): List<Binding> {
9696
(capacity, eventManager, shadowDomStrategy) => new rvf.ViewFactory(capacity, eventManager, shadowDomStrategy),
9797
[rvf.VIEW_POOL_CAPACITY, EventManager, ShadowDomStrategy]
9898
),
99-
bind(rvf.VIEW_POOL_CAPACITY).toValue(100000),
99+
bind(rvf.VIEW_POOL_CAPACITY).toValue(10000),
100100
ProtoViewFactory,
101101
// TODO(tbosch): We need an explicit factory here, as
102102
// we are getting errors in dart2js with mirrors...
103103
bind(ViewFactory).toFactory(
104104
(capacity) => new ViewFactory(capacity),
105105
[VIEW_POOL_CAPACITY]
106106
),
107-
bind(VIEW_POOL_CAPACITY).toValue(100000),
107+
bind(VIEW_POOL_CAPACITY).toValue(10000),
108108
Compiler,
109109
CompilerCache,
110110
TemplateResolver,

‎modules/angular2/src/core/compiler/view_factory.js‎

Lines changed: 20 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -12,37 +12,38 @@ export const VIEW_POOL_CAPACITY = 'ViewFactory.viewPoolCapacity';
1212

1313
@Injectable()
1414
export class ViewFactory {
15-
_poolCapacity:number;
16-
_pooledViews:List<viewModule.AppView>;
15+
_poolCapacityPerProtoView:number;
16+
_pooledViewsPerProtoView:Map<vieModule.ProtoView, List<viewModule.AppView>>;
1717

18-
constructor(@Inject(VIEW_POOL_CAPACITY) capacity) {
19-
this._poolCapacity = capacity;
20-
this._pooledViews = ListWrapper.create();
18+
constructor(@Inject(VIEW_POOL_CAPACITY) poolCapacityPerProtoView) {
19+
this._poolCapacityPerProtoView = poolCapacityPerProtoView;
20+
this._pooledViewsPerProtoView = MapWrapper.create();
2121
}
2222

2323
getView(protoView:viewModule.AppProtoView):viewModule.AppView {
24-
// TODO(tbosch): benchmark this scanning of views and maybe
25-
// replace it with a fancy LRU Map/List combination...
26-
var view;
27-
for (var i=this._pooledViews.length-1; i>=0; i--) {
28-
var pooledView = this._pooledViews[i];
29-
if (pooledView.proto === protoView) {
30-
view = ListWrapper.removeAt(this._pooledViews, i);
24+
var pooledViews = MapWrapper.get(this._pooledViewsPerProtoView, protoView);
25+
if (isPresent(pooledViews)) {
26+
var result = ListWrapper.removeLast(pooledViews);
27+
if (pooledViews.length === 0) {
28+
MapWrapper.delete(this._pooledViewsPerProtoView, protoView);
3129
}
30+
return result;
3231
}
33-
if (isBlank(view)) {
34-
view = this._createView(protoView);
35-
}
36-
return view;
32+
return this._createView(protoView);
3733
}
3834

3935
returnView(view:viewModule.AppView) {
4036
if (view.hydrated()) {
4137
throw new BaseException('Only dehydrated Views can be put back into the pool!');
4238
}
43-
ListWrapper.push(this._pooledViews, view);
44-
while (this._pooledViews.length > this._poolCapacity) {
45-
ListWrapper.removeAt(this._pooledViews, 0);
39+
var protoView = view.proto;
40+
var pooledViews = MapWrapper.get(this._pooledViewsPerProtoView, protoView);
41+
if (isBlank(pooledViews)) {
42+
pooledViews = [];
43+
MapWrapper.set(this._pooledViewsPerProtoView, protoView, pooledViews);
44+
}
45+
if (pooledViews.length < this._poolCapacityPerProtoView) {
46+
ListWrapper.push(pooledViews, view);
4647
}
4748
}
4849

‎modules/angular2/src/render/dom/view/view_factory.js‎

Lines changed: 21 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -18,41 +18,43 @@ export const VIEW_POOL_CAPACITY = 'render.ViewFactory.viewPoolCapacity';
1818

1919
@Injectable()
2020
export class ViewFactory {
21-
_poolCapacity:number;
22-
_pooledViews:List<viewModule.RenderView>;
21+
_poolCapacityPerProtoView:number;
22+
_pooledViewsPerProtoView:Map<pvModule.RenderProtoView, List<viewModule.RenderView>>;
2323
_eventManager:EventManager;
2424
_shadowDomStrategy:ShadowDomStrategy;
2525

26-
constructor(@Inject(VIEW_POOL_CAPACITY) capacity, eventManager:EventManager, shadowDomStrategy:ShadowDomStrategy) {
27-
this._poolCapacity = capacity;
28-
this._pooledViews = ListWrapper.create();
26+
constructor(@Inject(VIEW_POOL_CAPACITY) poolCapacityPerProtoView,
27+
eventManager:EventManager, shadowDomStrategy:ShadowDomStrategy) {
28+
this._poolCapacityPerProtoView = poolCapacityPerProtoView;
29+
this._pooledViewsPerProtoView = MapWrapper.create();
2930
this._eventManager = eventManager;
3031
this._shadowDomStrategy = shadowDomStrategy;
3132
}
3233

3334
getView(protoView:pvModule.RenderProtoView):viewModule.RenderView {
34-
// TODO(tbosch): benchmark this scanning of views and maybe
35-
// replace it with a fancy LRU Map/List combination...
36-
var view;
37-
for (var i=this._pooledViews.length-1; i>=0; i--) {
38-
var pooledView = this._pooledViews[i];
39-
if (pooledView.proto === protoView) {
40-
view = ListWrapper.removeAt(this._pooledViews, i);
35+
var pooledViews = MapWrapper.get(this._pooledViewsPerProtoView, protoView);
36+
if (isPresent(pooledViews)) {
37+
var result = ListWrapper.removeLast(pooledViews);
38+
if (pooledViews.length === 0) {
39+
MapWrapper.delete(this._pooledViewsPerProtoView, protoView);
4140
}
41+
return result;
4242
}
43-
if (isBlank(view)) {
44-
view = this._createView(protoView);
45-
}
46-
return view;
43+
return this._createView(protoView);
4744
}
4845

4946
returnView(view:viewModule.RenderView) {
5047
if (view.hydrated()) {
5148
view.dehydrate();
5249
}
53-
ListWrapper.push(this._pooledViews, view);
54-
while (this._pooledViews.length > this._poolCapacity) {
55-
ListWrapper.removeAt(this._pooledViews, 0);
50+
var protoView = view.proto;
51+
var pooledViews = MapWrapper.get(this._pooledViewsPerProtoView, protoView);
52+
if (isBlank(pooledViews)) {
53+
pooledViews = [];
54+
MapWrapper.set(this._pooledViewsPerProtoView, protoView, pooledViews);
55+
}
56+
if (pooledViews.length < this._poolCapacityPerProtoView) {
57+
ListWrapper.push(pooledViews, view);
5658
}
5759
}
5860

‎modules/angular2/test/core/compiler/view_factory_spec.js‎

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,73 @@
1+
import {describe, ddescribe, it, iit, xit, xdescribe, expect, beforeEach, el} from 'angular2/test_lib';
2+
3+
import {ViewFactory} from 'angular2/src/core/compiler/view_factory';
4+
import {AppProtoView, AppView} from 'angular2/src/core/compiler/view';
5+
import {dynamicChangeDetection} from 'angular2/change_detection';
6+
7+
export function main() {
8+
function createViewFactory({capacity}):ViewFactory {
9+
return new ViewFactory(capacity);
10+
}
11+
12+
function createPv() {
13+
return new AppProtoView(null,
14+
null,
15+
dynamicChangeDetection.createProtoChangeDetector('dummy', null));
16+
}
17+
18+
describe('RenderViewFactory', () => {
19+
it('should create views', () => {
20+
var pv = createPv();
21+
var vf = createViewFactory({
22+
capacity: 1
23+
});
24+
expect(vf.getView(pv) instanceof AppView).toBe(true);
25+
});
26+
27+
describe('caching', () => {
28+
29+
it('should support multiple AppProtoViews', () => {
30+
var capacity;
31+
var pv1 = createPv();
32+
var pv2 = createPv();
33+
var vf = createViewFactory({ capacity: 2 });
34+
var view1 = vf.getView(pv1);
35+
var view2 = vf.getView(pv2);
36+
vf.returnView(view1);
37+
vf.returnView(view2);
38+
39+
expect(vf.getView(pv1)).toBe(view1);
40+
expect(vf.getView(pv2)).toBe(view2);
41+
});
42+
43+
it('should reuse the newest view that has been returned', () => {
44+
var capacity;
45+
var pv = createPv();
46+
var vf = createViewFactory({ capacity: 2 });
47+
var view1 = vf.getView(pv);
48+
var view2 = vf.getView(pv);
49+
vf.returnView(view1);
50+
vf.returnView(view2);
51+
52+
expect(vf.getView(pv)).toBe(view2);
53+
});
54+
55+
it('should not add views when the capacity has been reached', () => {
56+
var capacity;
57+
var pv = createPv();
58+
var vf = createViewFactory({ capacity: 2 });
59+
var view1 = vf.getView(pv);
60+
var view2 = vf.getView(pv);
61+
var view3 = vf.getView(pv);
62+
vf.returnView(view1);
63+
vf.returnView(view2);
64+
vf.returnView(view3);
65+
66+
expect(vf.getView(pv)).toBe(view2);
67+
expect(vf.getView(pv)).toBe(view1);
68+
});
69+
70+
});
71+
72+
});
73+
}

‎modules/angular2/test/render/dom/view/view_factory_spec.js‎

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,75 @@
1+
import {describe, ddescribe, it, iit, xit, xdescribe, expect, beforeEach, el} from 'angular2/test_lib';
2+
3+
import {ViewFactory} from 'angular2/src/render/dom/view/view_factory';
4+
import {RenderProtoView} from 'angular2/src/render/dom/view/proto_view';
5+
import {RenderView} from 'angular2/src/render/dom/view/view';
6+
7+
export function main() {
8+
function createViewFactory({capacity}):ViewFactory {
9+
return new ViewFactory(capacity, null, null);
10+
}
11+
12+
function createPv() {
13+
return new RenderProtoView({
14+
element: el('<div></div>'),
15+
isRootView: false,
16+
elementBinders: []
17+
});
18+
}
19+
20+
describe('RenderViewFactory', () => {
21+
it('should create views', () => {
22+
var pv = createPv();
23+
var vf = createViewFactory({
24+
capacity: 1
25+
});
26+
expect(vf.getView(pv) instanceof RenderView).toBe(true);
27+
});
28+
29+
describe('caching', () => {
30+
31+
it('should support multiple RenderProtoViews', () => {
32+
var capacity;
33+
var pv1 = createPv();
34+
var pv2 = createPv();
35+
var vf = createViewFactory({ capacity: 2 });
36+
var view1 = vf.getView(pv1);
37+
var view2 = vf.getView(pv2);
38+
vf.returnView(view1);
39+
vf.returnView(view2);
40+
41+
expect(vf.getView(pv1)).toBe(view1);
42+
expect(vf.getView(pv2)).toBe(view2);
43+
});
44+
45+
it('should reuse the newest view that has been returned', () => {
46+
var capacity;
47+
var pv = createPv();
48+
var vf = createViewFactory({ capacity: 2 });
49+
var view1 = vf.getView(pv);
50+
var view2 = vf.getView(pv);
51+
vf.returnView(view1);
52+
vf.returnView(view2);
53+
54+
expect(vf.getView(pv)).toBe(view2);
55+
});
56+
57+
it('should not add views when the capacity has been reached', () => {
58+
var capacity;
59+
var pv = createPv();
60+
var vf = createViewFactory({ capacity: 2 });
61+
var view1 = vf.getView(pv);
62+
var view2 = vf.getView(pv);
63+
var view3 = vf.getView(pv);
64+
vf.returnView(view1);
65+
vf.returnView(view2);
66+
vf.returnView(view3);
67+
68+
expect(vf.getView(pv)).toBe(view2);
69+
expect(vf.getView(pv)).toBe(view1);
70+
});
71+
72+
});
73+
74+
});
75+
}

‎modules/examples/src/hello_world/index_static.js‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -202,7 +202,7 @@ function setup() {
202202
});
203203

204204
reflector.registerType(rvf.VIEW_POOL_CAPACITY, {
205-
"factory": () => 100000,
205+
"factory": () => 10000,
206206
"parameters": [],
207207
"annotations": []
208208
});
@@ -222,7 +222,7 @@ function setup() {
222222
});
223223

224224
reflector.registerType(VIEW_POOL_CAPACITY, {
225-
"factory": () => 100000,
225+
"factory": () => 10000,
226226
"parameters": [],
227227
"annotations": []
228228
});
@@ -261,7 +261,7 @@ function setup() {
261261
});
262262

263263
reflector.registerType(rvf.VIEW_POOL_CAPACITY, {
264-
"factory": () => 100000,
264+
"factory": () => 10000,
265265
"parameters": [],
266266
"annotations": []
267267
});
@@ -281,7 +281,7 @@ function setup() {
281281
});
282282

283283
reflector.registerType(VIEW_POOL_CAPACITY, {
284-
"factory": () => 100000,
284+
"factory": () => 10000,
285285
"parameters": [],
286286
"annotations": []
287287
});

0 commit comments

Comments
 (0)