Skip to content

Commit e858feb

Browse files
raju-optiraju-opti
committed
[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 e858feb

File tree

10 files changed

+185
-55
lines changed

10 files changed

+185
-55
lines changed

‎.github/pull_request_template.md‎

Lines changed: 0 additions & 10 deletions
This file was deleted.

‎lib/feature_toggle.ts‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,4 +31,6 @@
3131
* flag and all associated checks can be removed from the codebase.
3232
*/
3333

34-
export const holdout = () => true;
34+
export const holdout = () => false as const;
35+
36+
export type IfActive<T extends () => boolean, Y, N = unknown> = ReturnType<T> extends true ? Y : N;

‎lib/notification_center/type.ts‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,14 +26,16 @@ import {
2626
} from '../shared_types';
2727
import { DecisionSource } from '../utils/enums';
2828
import { Nullable } from '../utils/type';
29+
import { holdout, IfActive } from '../feature_toggle';
2930

3031
export type UserEventListenerPayload = {
3132
userId: string;
3233
attributes?: UserAttributes;
3334
}
3435

3536
export type ActivateListenerPayload = UserEventListenerPayload & {
36-
experiment: Experiment | Holdout | null;
37+
experiment: Experiment | null;
38+
holdout: IfActive<typeof holdout, Holdout | null>;
3739
variation: Variation | null;
3840
logEvent: LogEvent;
3941
}

‎lib/optimizely/index.spec.ts‎

Lines changed: 56 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ import { newErrorDecision } from '../optimizely_decision';
3131
import { ImpressionEvent } from '../event_processor/event_builder/user_event';
3232
import { OptimizelyDecideOption } from '../shared_types';
3333
import { NOTIFICATION_TYPES, DECISION_NOTIFICATION_TYPES } from '../notification_center/type';
34+
import { a } from 'vitest/dist/chunks/suite.B2jumIFP.js';
3435

3536

3637
const holdoutData = [
@@ -249,7 +250,8 @@ describe('Optimizely', () => {
249250
let projectConfig: any;
250251
let optimizely: any;
251252
let decisionService: any;
252-
let notificationSpy: any;
253+
let flagNotificationSpy: any;
254+
let activateNotificationSpy: any;
253255
let eventProcessor: any;
254256

255257
beforeEach(() => {
@@ -282,10 +284,16 @@ describe('Optimizely', () => {
282284
decisionService = optimizely.decisionService;
283285

284286
// Setup notification spy
285-
notificationSpy = vi.fn();
287+
flagNotificationSpy = vi.fn();
286288
optimizely.notificationCenter.addNotificationListener(
287289
NOTIFICATION_TYPES.DECISION,
288-
notificationSpy
290+
flagNotificationSpy
291+
);
292+
293+
activateNotificationSpy = vi.fn();
294+
optimizely.notificationCenter.addNotificationListener(
295+
NOTIFICATION_TYPES.ACTIVATE,
296+
activateNotificationSpy
289297
);
290298
});
291299

@@ -408,7 +416,7 @@ describe('Optimizely', () => {
408416
expect(decision.ruleKey).toBe('holdout_test_key');
409417

410418
// Verify decision notification was sent
411-
expect(notificationSpy).toHaveBeenCalledWith({
419+
expect(flagNotificationSpy).toHaveBeenCalledWith({
412420
type: DECISION_NOTIFICATION_TYPES.FLAG,
413421
userId: 'test_user',
414422
attributes: { country: 'US' },
@@ -422,6 +430,14 @@ describe('Optimizely', () => {
422430
decisionEventDispatched: true,
423431
}),
424432
});
433+
434+
expect(activateNotificationSpy).toHaveBeenCalledWith(expect.objectContaining({
435+
experiment: null,
436+
holdout: projectConfig.holdouts[0],
437+
userId: 'test_user',
438+
attributes: { country: 'US' },
439+
variation: projectConfig.holdouts[0].variations[0]
440+
}));
425441
});
426442

427443
it('should handle holdout with included flags', async () => {
@@ -455,7 +471,7 @@ describe('Optimizely', () => {
455471
expect(decision.variationKey).toBe('holdout_variation_key');
456472

457473
// Verify notification shows holdout details
458-
expect(notificationSpy).toHaveBeenCalledWith({
474+
expect(flagNotificationSpy).toHaveBeenCalledWith({
459475
type: DECISION_NOTIFICATION_TYPES.FLAG,
460476
userId: 'test_user',
461477
attributes: { country: 'US' },
@@ -465,6 +481,14 @@ describe('Optimizely', () => {
465481
ruleKey: 'holdout_test_key',
466482
}),
467483
});
484+
485+
expect(activateNotificationSpy).toHaveBeenCalledWith(expect.objectContaining({
486+
experiment: null,
487+
holdout: modifiedHoldout,
488+
userId: 'test_user',
489+
attributes: { country: 'US' },
490+
variation: modifiedHoldout.variations[0]
491+
}));
468492
});
469493

470494
it('should handle holdout with excluded flags', async () => {
@@ -499,7 +523,7 @@ describe('Optimizely', () => {
499523
expect(decision.variationKey).toBe('variation_3');
500524

501525
// Verify notification shows normal experiment details (not holdout)
502-
expect(notificationSpy).toHaveBeenCalledWith({
526+
expect(flagNotificationSpy).toHaveBeenCalledWith({
503527
type: DECISION_NOTIFICATION_TYPES.FLAG,
504528
userId: 'test_user',
505529
attributes: { country: 'BD', age: 80 },
@@ -509,6 +533,14 @@ describe('Optimizely', () => {
509533
ruleKey: 'exp_3',
510534
}),
511535
});
536+
537+
expect(activateNotificationSpy).toHaveBeenCalledWith(expect.objectContaining({
538+
experiment: projectConfig.experimentKeyMap['exp_3'],
539+
holdout: null,
540+
userId: 'test_user',
541+
attributes: { country: 'BD', age: 80 },
542+
variation: projectConfig.variationIdMap['5003']
543+
}));
512544
});
513545

514546
it('should handle multiple holdouts with correct priority', async () => {
@@ -568,7 +600,7 @@ describe('Optimizely', () => {
568600
expect(decision.variationKey).toBe('holdout_variation_key_2');
569601

570602
// Verify notification shows details of selected holdout
571-
expect(notificationSpy).toHaveBeenCalledWith({
603+
expect(flagNotificationSpy).toHaveBeenCalledWith({
572604
type: DECISION_NOTIFICATION_TYPES.FLAG,
573605
userId: 'test_user',
574606
attributes: { country: 'US' },
@@ -578,6 +610,14 @@ describe('Optimizely', () => {
578610
ruleKey: 'holdout_test_key_2',
579611
}),
580612
});
613+
614+
expect(activateNotificationSpy).toHaveBeenCalledWith(expect.objectContaining({
615+
experiment: null,
616+
holdout: holdout2,
617+
userId: 'test_user',
618+
attributes: { country: 'US' },
619+
variation: holdout2.variations[0]
620+
}));
581621
});
582622

583623
it('should respect sendFlagDecisions setting for holdout events - false', async () => {
@@ -744,7 +784,7 @@ describe('Optimizely', () => {
744784
expect(typeof decision.variables).toBe('object');
745785

746786
// Verify notification includes variable information
747-
expect(notificationSpy).toHaveBeenCalledWith({
787+
expect(flagNotificationSpy).toHaveBeenCalledWith({
748788
type: DECISION_NOTIFICATION_TYPES.FLAG,
749789
userId: 'test_user',
750790
attributes: { country: 'US' },
@@ -754,6 +794,14 @@ describe('Optimizely', () => {
754794
enabled: false,
755795
}),
756796
});
797+
798+
expect(activateNotificationSpy).toHaveBeenCalledWith(expect.objectContaining({
799+
experiment: null,
800+
holdout: projectConfig.holdouts[0],
801+
userId: 'test_user',
802+
attributes: { country: 'US' },
803+
variation: projectConfig.holdouts[0].variations[0]
804+
}));
757805
});
758806

759807
it('should handle disable decision event option for holdout', async () => {

‎lib/optimizely/index.tests.js‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,7 @@ import {
6767

6868
import { USER_BUCKETED_INTO_EXPERIMENT_IN_GROUP } from '../core/bucketer';
6969
import { resolvablePromise } from '../utils/promise/resolvablePromise';
70+
import { holdout } from '../feature_toggle';
7071

7172
var LOG_LEVEL = enums.LOG_LEVEL;
7273
var DECISION_SOURCES = enums.DECISION_SOURCES;
@@ -2281,6 +2282,7 @@ describe('lib/optimizely', function() {
22812282
var instanceExperiments = optlyInstance.projectConfigManager.getConfig().experiments;
22822283
var expectedArgument = {
22832284
experiment: instanceExperiments[0],
2285+
holdout: null,
22842286
userId: 'testUser',
22852287
attributes: undefined,
22862288
variation: instanceExperiments[0].variations[1],
@@ -2351,6 +2353,7 @@ describe('lib/optimizely', function() {
23512353
var instanceExperiments = optlyInstance.projectConfigManager.getConfig().experiments;
23522354
var expectedArgument = {
23532355
experiment: instanceExperiments[0],
2356+
holdout: null,
23542357
userId: 'testUser',
23552358
attributes: attributes,
23562359
variation: instanceExperiments[0].variations[1],

‎lib/optimizely/index.ts‎

Lines changed: 18 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ import {
3737
OptimizelyDecision,
3838
Client,
3939
UserProfileServiceAsync,
40+
isHoldout,
4041
} from '../shared_types';
4142
import { newErrorDecision } from '../optimizely_decision';
4243
import OptimizelyUserContext from '../optimizely_user_context';
@@ -62,7 +63,7 @@ import {
6263
import { Fn, Maybe, OpType } from '../utils/type';
6364
import { resolvablePromise } from '../utils/promise/resolvablePromise';
6465

65-
import { NOTIFICATION_TYPES, DecisionNotificationType, DECISION_NOTIFICATION_TYPES } from '../notification_center/type';
66+
import { NOTIFICATION_TYPES, DecisionNotificationType, DECISION_NOTIFICATION_TYPES, ActivateListenerPayload } from '../notification_center/type';
6667
import {
6768
FEATURE_NOT_IN_DATAFILE,
6869
INVALID_INPUT_FORMAT,
@@ -382,17 +383,29 @@ export default class Optimizely extends BaseService implements Client {
382383
this.eventProcessor.process(impressionEvent);
383384

384385
const logEvent = buildLogEvent([impressionEvent]);
385-
this.notificationCenter.sendNotifications(NOTIFICATION_TYPES.ACTIVATE, {
386-
experiment: decisionObj.experiment,
386+
387+
const activateNotificationPayload: ActivateListenerPayload = {
388+
experiment: null,
389+
holdout: null,
387390
userId: userId,
388391
attributes: attributes,
389392
variation: decisionObj.variation,
390393
logEvent,
391-
});
394+
};
395+
396+
if (decisionObj.experiment) {
397+
if (isHoldout(decisionObj.experiment)) {
398+
activateNotificationPayload.holdout = decisionObj.experiment;
399+
} else {
400+
activateNotificationPayload.experiment = decisionObj.experiment;
401+
}
402+
}
403+
404+
this.notificationCenter.sendNotifications(NOTIFICATION_TYPES.ACTIVATE, activateNotificationPayload);
392405
}
393406

394407
/**
395-
* Sends conversion event to Optimizely.
408+
* Sends conversion event to Optimizely.`
396409
* @param {string} eventKey
397410
* @param {string} userId
398411
* @param {UserAttributes} attributes

‎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/shared_types.ts‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -181,6 +181,15 @@ export interface Holdout extends ExperimentCore {
181181
excludedFlags: string[];
182182
}
183183

184+
export function isHoldout(obj: Experiment | Holdout): obj is Holdout {
185+
// Holdout has 'status', 'includedFlags', and 'excludedFlags' properties
186+
return (
187+
(obj as Holdout).status !== undefined &&
188+
Array.isArray((obj as Holdout).includedFlags) &&
189+
Array.isArray((obj as Holdout).excludedFlags)
190+
);
191+
}
192+
184193
export enum VariableType {
185194
BOOLEAN = 'boolean',
186195
DOUBLE = 'double',

0 commit comments

Comments
 (0)