From 2af937dd9d73a51ff75f214031a012fec2cb2835 Mon Sep 17 00:00:00 2001 From: Kara Erickson Date: Wed, 14 Mar 2018 13:29:48 -0700 Subject: [PATCH] refactor(ivy): double size of DI bloom filter --- packages/core/src/render3/di.ts | 61 +++++++++++-------- .../core/src/render3/interfaces/injector.ts | 17 +++++- packages/core/test/render3/di_spec.ts | 39 ++++++++++-- 3 files changed, 86 insertions(+), 31 deletions(-) diff --git a/packages/core/src/render3/di.ts b/packages/core/src/render3/di.ts index c63ca32ad8ea0..fa90cc5cdb159 100644 --- a/packages/core/src/render3/di.ts +++ b/packages/core/src/render3/di.ts @@ -45,7 +45,7 @@ const NG_ELEMENT_ID = '__NG_ELEMENT_ID__'; * directives that will share slots, and thus, the fewer false positives when checking for * the existence of a directive. */ -const BLOOM_SIZE = 128; +const BLOOM_SIZE = 256; /** Counter used to generate unique IDs for directives. */ let nextNgElementId = 0; @@ -66,9 +66,9 @@ export function bloomAdd(injector: LInjector, type: Type): void { id = (type as any)[NG_ELEMENT_ID] = nextNgElementId++; } - // We only have BLOOM_SIZE (128) slots in our bloom filter (4 buckets * 32 bits each), - // so all unique IDs must be modulo-ed into a number from 0 - 127 to fit into the filter. - // This means that after 128, some directives will share slots, leading to some false positives + // We only have BLOOM_SIZE (256) slots in our bloom filter (8 buckets * 32 bits each), + // so all unique IDs must be modulo-ed into a number from 0 - 255 to fit into the filter. + // This means that after 255, some directives will share slots, leading to some false positives // when checking for a directive's presence. const bloomBit = id % BLOOM_SIZE; @@ -78,20 +78,14 @@ export function bloomAdd(injector: LInjector, type: Type): void { const mask = 1 << bloomBit; // Use the raw bloomBit number to determine which bloom filter bucket we should check - // e.g: bf0 = [0 - 31], bf1 = [32 - 63], bf2 = [64 - 95], bf3 = [96 - 127] - if (bloomBit < 64) { - if (bloomBit < 32) { - // Then use the mask to flip on the bit (0-31) associated with the directive in that bucket - injector.bf0 |= mask; - } else { - injector.bf1 |= mask; - } + // e.g: bf0 = [0 - 31], bf1 = [32 - 63], bf2 = [64 - 95], bf3 = [96 - 127], etc + if (bloomBit < 128) { + // Then use the mask to flip on the bit (0-31) associated with the directive in that bucket + bloomBit < 64 ? (bloomBit < 32 ? (injector.bf0 |= mask) : (injector.bf1 |= mask)) : + (bloomBit < 96 ? (injector.bf2 |= mask) : (injector.bf3 |= mask)); } else { - if (bloomBit < 96) { - injector.bf2 |= mask; - } else { - injector.bf3 |= mask; - } + bloomBit < 192 ? (bloomBit < 160 ? (injector.bf4 |= mask) : (injector.bf5 |= mask)) : + (bloomBit < 224 ? (injector.bf6 |= mask) : (injector.bf7 |= mask)); } } @@ -119,10 +113,18 @@ export function getOrCreateNodeInjectorForNode(node: LElementNode | LContainerNo bf1: 0, bf2: 0, bf3: 0, + bf4: 0, + bf5: 0, + bf6: 0, + bf7: 0, cbf0: parentInjector == null ? 0 : parentInjector.cbf0 | parentInjector.bf0, cbf1: parentInjector == null ? 0 : parentInjector.cbf1 | parentInjector.bf1, cbf2: parentInjector == null ? 0 : parentInjector.cbf2 | parentInjector.bf2, cbf3: parentInjector == null ? 0 : parentInjector.cbf3 | parentInjector.bf3, + cbf4: parentInjector == null ? 0 : parentInjector.cbf4 | parentInjector.bf4, + cbf5: parentInjector == null ? 0 : parentInjector.cbf5 | parentInjector.bf5, + cbf6: parentInjector == null ? 0 : parentInjector.cbf6 | parentInjector.bf6, + cbf7: parentInjector == null ? 0 : parentInjector.cbf7 | parentInjector.bf7, injector: null, templateRef: null, viewContainerRef: null, @@ -461,11 +463,17 @@ export function bloomFindPossibleInjector(startInjector: LInjector, bloomBit: nu // match. let injector: LInjector|null = startInjector; while (injector) { - // Our bloom filter size is 128 bits, which is four 32-bit bloom filter buckets: - // bf0 = [0 - 31], bf1 = [32 - 63], bf2 = [64 - 95], bf3 = [96 - 127] + // Our bloom filter size is 256 bits, which is eight 32-bit bloom filter buckets: + // bf0 = [0 - 31], bf1 = [32 - 63], bf2 = [64 - 95], bf3 = [96 - 127], etc. // Get the bloom filter value from the appropriate bucket based on the directive's bloomBit. - let value: number = bloomBit < 64 ? (bloomBit < 32 ? injector.bf0 : injector.bf1) : - (bloomBit < 96 ? injector.bf2 : injector.bf3); + let value: number; + if (bloomBit < 128) { + value = bloomBit < 64 ? (bloomBit < 32 ? injector.bf0 : injector.bf1) : + (bloomBit < 96 ? injector.bf2 : injector.bf3); + } else { + value = bloomBit < 192 ? (bloomBit < 160 ? injector.bf4 : injector.bf5) : + (bloomBit < 224 ? injector.bf6 : injector.bf7); + } // If the bloom filter value has the bit corresponding to the directive's bloomBit flipped on, // this injector is a potential match. @@ -474,9 +482,14 @@ export function bloomFindPossibleInjector(startInjector: LInjector, bloomBit: nu } // If the current injector does not have the directive, check the bloom filters for the ancestor - // injectors (cbf0 - cbf3). These filters capture *all* ancestor injectors. - value = bloomBit < 64 ? (bloomBit < 32 ? injector.cbf0 : injector.cbf1) : - (bloomBit < 96 ? injector.cbf2 : injector.cbf3); + // injectors (cbf0 - cbf7). These filters capture *all* ancestor injectors. + if (bloomBit < 128) { + value = bloomBit < 64 ? (bloomBit < 32 ? injector.cbf0 : injector.cbf1) : + (bloomBit < 96 ? injector.cbf2 : injector.cbf3); + } else { + value = bloomBit < 192 ? (bloomBit < 160 ? injector.cbf4 : injector.cbf5) : + (bloomBit < 224 ? injector.cbf6 : injector.cbf7); + } // If the ancestor bloom filter value has the bit corresponding to the directive, traverse up to // find the specific injector. If the ancestor bloom filter does not have the bit, we can abort. diff --git a/packages/core/src/render3/interfaces/injector.ts b/packages/core/src/render3/interfaces/injector.ts index 59d79c61f7c98..f321287cc4b31 100644 --- a/packages/core/src/render3/interfaces/injector.ts +++ b/packages/core/src/render3/interfaces/injector.ts @@ -35,9 +35,13 @@ export interface LInjector { * array at this level unless it's probable the directive is in it. * * - bf0: Check directive IDs 0-31 (IDs are % 128) - * - bf1: Check directive IDs 33-63 + * - bf1: Check directive IDs 32-63 * - bf2: Check directive IDs 64-95 * - bf3: Check directive IDs 96-127 + * - bf4: Check directive IDs 128-159 + * - bf5: Check directive IDs 160 - 191 + * - bf6: Check directive IDs 192 - 223 + * - bf7: Check directive IDs 224 - 255 * * See: https://en.wikipedia.org/wiki/Bloom_filter for more about bloom filters. */ @@ -45,9 +49,13 @@ export interface LInjector { bf1: number; bf2: number; bf3: number; + bf4: number; + bf5: number; + bf6: number; + bf7: number; /** - * cbf0 - cbf3 properties determine whether a directive is available through a + * cbf0 - cbf7 properties determine whether a directive is available through a * parent injector. They refer to the merged values of parent bloom filters. This * allows us to skip looking up the chain unless it's probable that directive exists * up the chain. @@ -56,6 +64,11 @@ export interface LInjector { cbf1: number; cbf2: number; cbf3: number; + cbf4: number; + cbf5: number; + cbf6: number; + cbf7: number; + injector: Injector|null; /** Stores the TemplateRef so subsequent injections of the TemplateRef get the same instance. */ diff --git a/packages/core/test/render3/di_spec.ts b/packages/core/test/render3/di_spec.ts index 05929439b2d26..a4563444b3908 100644 --- a/packages/core/test/render3/di_spec.ts +++ b/packages/core/test/render3/di_spec.ts @@ -487,23 +487,42 @@ describe('di', () => { di.bf1 = 0; di.bf2 = 0; di.bf3 = 0; + di.bf4 = 0; + di.bf5 = 0; + di.bf6 = 0; + di.bf7 = 0; + di.bf3 = 0; di.cbf0 = 0; di.cbf1 = 0; di.cbf2 = 0; di.cbf3 = 0; + di.cbf4 = 0; + di.cbf5 = 0; + di.cbf6 = 0; + di.cbf7 = 0; }); - function bloomState() { return [di.bf3, di.bf2, di.bf1, di.bf0]; } + function bloomState() { + return [di.bf7, di.bf6, di.bf5, di.bf4, di.bf3, di.bf2, di.bf1, di.bf0]; + } it('should add values', () => { bloomAdd(di, { __NG_ELEMENT_ID__: 0 } as any); - expect(bloomState()).toEqual([0, 0, 0, 1]); + expect(bloomState()).toEqual([0, 0, 0, 0, 0, 0, 0, 1]); bloomAdd(di, { __NG_ELEMENT_ID__: 32 + 1 } as any); - expect(bloomState()).toEqual([0, 0, 2, 1]); + expect(bloomState()).toEqual([0, 0, 0, 0, 0, 0, 2, 1]); bloomAdd(di, { __NG_ELEMENT_ID__: 64 + 2 } as any); - expect(bloomState()).toEqual([0, 4, 2, 1]); + expect(bloomState()).toEqual([0, 0, 0, 0, 0, 4, 2, 1]); bloomAdd(di, { __NG_ELEMENT_ID__: 96 + 3 } as any); - expect(bloomState()).toEqual([8, 4, 2, 1]); + expect(bloomState()).toEqual([0, 0, 0, 0, 8, 4, 2, 1]); + bloomAdd(di, { __NG_ELEMENT_ID__: 128 + 4 } as any); + expect(bloomState()).toEqual([0, 0, 0, 16, 8, 4, 2, 1]); + bloomAdd(di, { __NG_ELEMENT_ID__: 160 + 5 } as any); + expect(bloomState()).toEqual([0, 0, 32, 16, 8, 4, 2, 1]); + bloomAdd(di, { __NG_ELEMENT_ID__: 192 + 6 } as any); + expect(bloomState()).toEqual([0, 64, 32, 16, 8, 4, 2, 1]); + bloomAdd(di, { __NG_ELEMENT_ID__: 224 + 7 } as any); + expect(bloomState()).toEqual([128, 64, 32, 16, 8, 4, 2, 1]); }); it('should query values', () => { @@ -511,12 +530,22 @@ describe('di', () => { bloomAdd(di, { __NG_ELEMENT_ID__: 32 } as any); bloomAdd(di, { __NG_ELEMENT_ID__: 64 } as any); bloomAdd(di, { __NG_ELEMENT_ID__: 96 } as any); + bloomAdd(di, { __NG_ELEMENT_ID__: 127 } as any); + bloomAdd(di, { __NG_ELEMENT_ID__: 161 } as any); + bloomAdd(di, { __NG_ELEMENT_ID__: 188 } as any); + bloomAdd(di, { __NG_ELEMENT_ID__: 223 } as any); + bloomAdd(di, { __NG_ELEMENT_ID__: 255 } as any); expect(bloomFindPossibleInjector(di, 0)).toEqual(di); expect(bloomFindPossibleInjector(di, 1)).toEqual(null); expect(bloomFindPossibleInjector(di, 32)).toEqual(di); expect(bloomFindPossibleInjector(di, 64)).toEqual(di); expect(bloomFindPossibleInjector(di, 96)).toEqual(di); + expect(bloomFindPossibleInjector(di, 127)).toEqual(di); + expect(bloomFindPossibleInjector(di, 161)).toEqual(di); + expect(bloomFindPossibleInjector(di, 188)).toEqual(di); + expect(bloomFindPossibleInjector(di, 223)).toEqual(di); + expect(bloomFindPossibleInjector(di, 255)).toEqual(di); }); });