Fix for slowdown caused by initialfetch (microsoft#16969)

karthiknadig · kimadeline · web-flow · commit a75688b2d0ab · 2021-08-16T14:06:19.000-07:00
* Fix for slowdown caused by initialfetch * Fix tests * Use in memory cache to hold on to values for a given session. * Revert "Use in memory cache to hold on to values for a given session." This reverts commit 82c62ba. * Comment clarifying not awaiting on initialFetch * Update src/client/common/experiments/service.ts Co-authored-by: Kim-Adeline Miguel <51720070+kimadeline@users.noreply.github.com> Co-authored-by: Kim-Adeline Miguel <51720070+kimadeline@users.noreply.github.com>
diff --git a/news/2 Fixes/16959.md b/news/2 Fixes/16959.md
@@ -0,0 +1 @@
+In experiments service don't always `await` on `initialfetch` which can be slow depending on the network.
diff --git a/src/client/common/experiments/service.ts b/src/client/common/experiments/service.ts
@@ -82,7 +82,21 @@ export class ExperimentService implements IExperimentService {
     public async activate(): Promise<void> {
         if (this.experimentationService) {
             await this.experimentationService.initializePromise;
-            await this.experimentationService.initialFetch;
+
+            const experiments = this.globalState.get<{ features: string[] }>(EXP_MEMENTO_KEY, { features: [] });
+            if (experiments.features.length === 0) {
+                // Only await on this if we don't have anything in cache.
+                // This means that we start the session with partial experiment info.
+                // We accept this as a compromise to avoid delaying startup.
+
+                // In the case where we don't wait on this promise. If the experiment info changes,
+                // those changes will be applied in the next session. This is controlled internally
+                // in the tas-client via `overrideInMemoryFeatures` value that is passed to
+                // `getFeaturesAsync`. At the time of writing this comment the value of
+                // `overrideInMemoryFeatures` was always passed in as `false`. So, the experiment
+                // states did not change mid way.
+                await this.experimentationService.initialFetch;
+            }
         }
         sendOptInOptOutTelemetry(this._optInto, this._optOutFrom, this.appEnvironment.packageJson);
     }
@@ -122,7 +136,7 @@ export class ExperimentService implements IExperimentService {
             return undefined;
         }
 
-        return this.experimentationService.getTreatmentVariableAsync(EXP_CONFIG_ID, experiment, true);
+        return this.experimentationService.getTreatmentVariable<T>(EXP_CONFIG_ID, experiment);
     }
 
     private logExperiments() {
diff --git a/src/test/common/experiments/service.unit.test.ts b/src/test/common/experiments/service.unit.test.ts
@@ -327,12 +327,12 @@ suite('Experimentation service', () => {
 
     suite('Experiment value retrieval', () => {
         const experiment = 'Test Experiment - experiment';
-        let getTreatmentVariableAsyncStub: sinon.SinonStub;
+        let getTreatmentVariableStub: sinon.SinonStub;
 
         setup(() => {
-            getTreatmentVariableAsyncStub = sinon.stub().returns(Promise.resolve('value'));
+            getTreatmentVariableStub = sinon.stub().returns(Promise.resolve('value'));
             sinon.stub(tasClient, 'getExperimentationService').returns(({
-                getTreatmentVariableAsync: getTreatmentVariableAsyncStub,
+                getTreatmentVariable: getTreatmentVariableStub,
             } as unknown) as tasClient.IExperimentationService);
 
             configureApplicationEnvironment('stable', extensionVersion);
@@ -350,7 +350,7 @@ suite('Experimentation service', () => {
             const result = await experimentService.getExperimentValue(experiment);
 
             assert.strictEqual(result, 'value');
-            sinon.assert.calledOnce(getTreatmentVariableAsyncStub);
+            sinon.assert.calledOnce(getTreatmentVariableStub);
         });
 
         test('If the experiment setting is disabled, getExperimentValue should return undefined', async () => {
@@ -365,7 +365,7 @@ suite('Experimentation service', () => {
             const result = await experimentService.getExperimentValue(experiment);
 
             assert.isUndefined(result);
-            sinon.assert.notCalled(getTreatmentVariableAsyncStub);
+            sinon.assert.notCalled(getTreatmentVariableStub);
         });
 
         test('If the opt-out setting contains "All", getExperimentValue should return undefined', async () => {
@@ -380,10 +380,10 @@ suite('Experimentation service', () => {
             const result = await experimentService.getExperimentValue(experiment);
 
             assert.isUndefined(result);
-            sinon.assert.notCalled(getTreatmentVariableAsyncStub);
+            sinon.assert.notCalled(getTreatmentVariableStub);
         });
 
-        test('If the opt-out setting contains the experiment name, igetExperimentValue should return undefined', async () => {
+        test('If the opt-out setting contains the experiment name, getExperimentValue should return undefined', async () => {
             configureSettings(true, [], [experiment]);
 
             const experimentService = new ExperimentService(
@@ -395,7 +395,7 @@ suite('Experimentation service', () => {
             const result = await experimentService.getExperimentValue(experiment);
 
             assert.isUndefined(result);
-            sinon.assert.notCalled(getTreatmentVariableAsyncStub);
+            sinon.assert.notCalled(getTreatmentVariableStub);
         });
     });
 

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1 @@`
	`1`	+In experiments service don't always `await` on `initialfetch` which can be slow depending on the network.