Skip to content

Commit fb5e012

Browse files
authored
[FSSDK-11787] fix config parsing performance (#1080)
Currently, variationIdMap is being populated as follows: ``` projectConfig.variationIdMap = { ...projectConfig.variationIdMap, ...keyBy(experiment.variations, 'id') }; ``` This creates a new objects for each experiment unnecessarily. This causes large slowdown in project config parsing for datafiles containing a large number of experiments containing a large number of variations. This commit makes this more efficient.
1 parent 017e10b commit fb5e012

File tree

3 files changed

+93
-30
lines changed

3 files changed

+93
-30
lines changed

lib/project_config/project_config.ts

Lines changed: 7 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313
* See the License for the specific language governing permissions and
1414
* limitations under the License.
1515
*/
16-
import { find, objectEntries, objectValues, keyBy } from '../utils/fns';
16+
import { find, objectEntries, objectValues, keyBy, assignBy } from '../utils/fns';
1717

1818
import { FEATURE_VARIABLE_TYPES } from '../utils/enums';
1919
import configValidator from '../utils/config_validator';
@@ -180,10 +180,10 @@ export const createProjectConfig = function(datafileObj?: JSON, datafileStr: str
180180
audience.conditions = JSON.parse(audience.conditions as string);
181181
});
182182

183-
projectConfig.audiencesById = {
184-
...keyBy(projectConfig.audiences, 'id'),
185-
...keyBy(projectConfig.typedAudiences, 'id'),
186-
}
183+
184+
projectConfig.audiencesById = {};
185+
assignBy(projectConfig.audiences, 'id', projectConfig.audiencesById);
186+
assignBy(projectConfig.typedAudiences, 'id', projectConfig.audiencesById);
187187

188188
projectConfig.attributes = projectConfig.attributes || [];
189189
projectConfig.attributeKeyMap = {};
@@ -267,11 +267,7 @@ export const createProjectConfig = function(datafileObj?: JSON, datafileStr: str
267267
// Creates { <variationKey>: <variation> } map inside of the experiment
268268
experiment.variationKeyMap = keyBy(experiment.variations, 'key');
269269

270-
// Creates { <variationId>: { key: <variationKey>, id: <variationId> } } mapping for quick lookup
271-
projectConfig.variationIdMap = {
272-
...projectConfig.variationIdMap,
273-
...keyBy(experiment.variations, 'id')
274-
};
270+
assignBy(experiment.variations, 'id', projectConfig.variationIdMap);
275271

276272
objectValues(experiment.variationKeyMap || {}).forEach(variation => {
277273
if (variation.variables) {
@@ -373,10 +369,7 @@ const parseHoldoutsConfig = (projectConfig: ProjectConfig): void => {
373369

374370
holdout.variationKeyMap = keyBy(holdout.variations, 'key');
375371

376-
projectConfig.variationIdMap = {
377-
...projectConfig.variationIdMap,
378-
...keyBy(holdout.variations, 'id'),
379-
};
372+
assignBy(holdout.variations, 'id', projectConfig.variationIdMap);
380373

381374
if (holdout.includedFlags.length === 0) {
382375
projectConfig.globalHoldouts.push(holdout);

lib/utils/fns/index.spec.ts

Lines changed: 74 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { describe, it, expect } from 'vitest';
22

3-
import { groupBy, objectEntries, objectValues, find, keyByUtil, sprintf } from '.'
3+
import { groupBy, objectEntries, objectValues, find, sprintf, keyBy, assignBy } from '.'
44

55
describe('utils', () => {
66
describe('groupBy', () => {
@@ -68,14 +68,86 @@ describe('utils', () => {
6868
{ key: 'baz', firstName: 'james', lastName: 'foxy' },
6969
]
7070

71-
expect(keyByUtil(input, item => item.key)).toEqual({
71+
expect(keyBy(input, 'key')).toEqual({
7272
foo: { key: 'foo', firstName: 'jordan', lastName: 'foo' },
7373
bar: { key: 'bar', firstName: 'jordan', lastName: 'bar' },
7474
baz: { key: 'baz', firstName: 'james', lastName: 'foxy' },
7575
})
7676
})
7777
})
7878

79+
describe('assignBy', () => {
80+
it('should assign array elements to an object using the specified key', () => {
81+
const input = [
82+
{ key: 'foo', firstName: 'jordan', lastName: 'foo' },
83+
{ key: 'bar', firstName: 'jordan', lastName: 'bar' },
84+
{ key: 'baz', firstName: 'james', lastName: 'foxy' },
85+
]
86+
const base = {}
87+
88+
assignBy(input, 'key', base)
89+
90+
expect(base).toEqual({
91+
foo: { key: 'foo', firstName: 'jordan', lastName: 'foo' },
92+
bar: { key: 'bar', firstName: 'jordan', lastName: 'bar' },
93+
baz: { key: 'baz', firstName: 'james', lastName: 'foxy' },
94+
})
95+
})
96+
97+
it('should append to an existing object', () => {
98+
const input = [
99+
{ key: 'foo', firstName: 'jordan', lastName: 'foo' },
100+
{ key: 'bar', firstName: 'jordan', lastName: 'bar' },
101+
]
102+
const base: any = { existing: 'value' }
103+
104+
assignBy(input, 'key', base)
105+
106+
expect(base).toEqual({
107+
existing: 'value',
108+
foo: { key: 'foo', firstName: 'jordan', lastName: 'foo' },
109+
bar: { key: 'bar', firstName: 'jordan', lastName: 'bar' },
110+
})
111+
})
112+
113+
it('should handle empty array', () => {
114+
const base: any = { existing: 'value' }
115+
116+
assignBy([], 'key', base)
117+
118+
expect(base).toEqual({ existing: 'value' })
119+
})
120+
121+
it('should handle null/undefined array', () => {
122+
const base: any = { existing: 'value' }
123+
124+
assignBy(null as any, 'key', base)
125+
expect(base).toEqual({ existing: 'value' })
126+
127+
assignBy(undefined as any, 'key', base)
128+
expect(base).toEqual({ existing: 'value' })
129+
})
130+
131+
it('should override existing values with the same key', () => {
132+
const input = [
133+
{ key: 'foo', firstName: 'jordan', lastName: 'updated' },
134+
{ key: 'bar', firstName: 'james', lastName: 'new' },
135+
]
136+
const base: any = {
137+
foo: { key: 'foo', firstName: 'john', lastName: 'original' },
138+
existing: 'value'
139+
}
140+
141+
assignBy(input, 'key', base)
142+
143+
expect(base).toEqual({
144+
existing: 'value',
145+
foo: { key: 'foo', firstName: 'jordan', lastName: 'updated' },
146+
bar: { key: 'bar', firstName: 'james', lastName: 'new' },
147+
})
148+
})
149+
})
150+
79151
describe('sprintf', () => {
80152
it('sprintf(msg)', () => {
81153
expect(sprintf('this is my message')).toBe('this is my message')

lib/utils/fns/index.ts

Lines changed: 12 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -25,11 +25,19 @@ export function isSafeInteger(number: unknown): boolean {
2525
return typeof number == 'number' && Math.abs(number) <= MAX_SAFE_INTEGER_LIMIT;
2626
}
2727

28-
export function keyBy<K>(arr: K[], key: string): { [key: string]: K } {
28+
export function keyBy<K>(arr: K[], key: string): Record<string, K> {
2929
if (!arr) return {};
30-
return keyByUtil(arr, function(item) {
31-
// eslint-disable-next-line @typescript-eslint/no-explicit-any
32-
return (item as any)[key];
30+
31+
const base: Record<string, K> = {};
32+
assignBy(arr, key, base);
33+
return base;
34+
}
35+
36+
export function assignBy<K>(arr: K[], key: string, base: Record<string, K>): void {
37+
if (!arr) return;
38+
39+
arr.forEach((e) => {
40+
base[(e as any)[key]] = e;
3341
});
3442
}
3543

@@ -81,15 +89,6 @@ export function find<K>(arr: K[], cond: (arg: K) => boolean): K | undefined {
8189
return found;
8290
}
8391

84-
export function keyByUtil<K>(arr: K[], keyByFn: (item: K) => string): { [key: string]: K } {
85-
const map: { [key: string]: K } = {};
86-
arr.forEach(item => {
87-
const key = keyByFn(item);
88-
map[key] = item;
89-
});
90-
return map;
91-
}
92-
9392
// TODO[OASIS-6649]: Don't use any type
9493
// eslint-disable-next-line @typescript-eslint/no-explicit-any
9594
export function sprintf(format: string, ...args: any[]): string {
@@ -129,6 +128,5 @@ export default {
129128
objectValues,
130129
objectEntries,
131130
find,
132-
keyByUtil,
133131
sprintf,
134132
};

0 commit comments

Comments
 (0)