Skip to content

Commit 2ac0748

Browse files
committed
Simplify profile publishing flow.
Redux allows us to cleanly separate asynchronous work from synchronous state: Anything that happens asynchronously should happen in the action creators. The Redux state, and the selectors, should always synchronously reflect the "current truth" about the world. Our profile publishing code was calling the asynchronous "compress" function from within a selector. This change moves this into an action creator.
1 parent b0610db commit 2ac0748

7 files changed

Lines changed: 272 additions & 172 deletions

File tree

src/actions/publish.ts

Lines changed: 134 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -9,9 +9,9 @@ import type { SanitizeProfileResult } from 'firefox-profiler/profile-logic/sanit
99
import {
1010
getUploadGeneration,
1111
getSanitizedProfile,
12-
getSanitizedProfileData,
1312
getRemoveProfileInformation,
1413
getPrePublishedState,
14+
getSanitizedProfileEncodingState,
1515
} from 'firefox-profiler/selectors/publish';
1616
import {
1717
getDataSource,
@@ -36,7 +36,10 @@ import type {
3636
StartEndRange,
3737
ThreadIndex,
3838
State,
39+
Profile,
3940
} from 'firefox-profiler/types';
41+
import { compress } from 'firefox-profiler/utils/gz';
42+
import { serializeProfile } from 'firefox-profiler/profile-logic/process-profile';
4043

4144
export function updateSharingOption(
4245
slug: keyof CheckedSharingOptions,
@@ -49,6 +52,37 @@ export function updateSharingOption(
4952
};
5053
}
5154

55+
export function sanitizedProfileEncodingStarted(
56+
sanitizedProfile: Profile
57+
): Action {
58+
return {
59+
type: 'SANITIZED_PROFILE_ENCODING_STARTED',
60+
sanitizedProfile,
61+
};
62+
}
63+
64+
export function sanitizedProfileEncodingCompleted(
65+
sanitizedProfile: Profile,
66+
profileData: Blob
67+
): Action {
68+
return {
69+
type: 'SANITIZED_PROFILE_ENCODING_COMPLETED',
70+
sanitizedProfile,
71+
profileData,
72+
};
73+
}
74+
75+
export function sanitizedProfileEncodingFailed(
76+
sanitizedProfile: Profile,
77+
error: Error
78+
): Action {
79+
return {
80+
type: 'SANITIZED_PROFILE_ENCODING_FAILED',
81+
sanitizedProfile,
82+
error,
83+
};
84+
}
85+
5286
export function uploadCompressionStarted(abortFunction: () => void): Action {
5387
return {
5488
type: 'UPLOAD_COMPRESSION_STARTED',
@@ -198,6 +232,90 @@ async function persistJustUploadedProfileInformationToDb(
198232
}
199233
}
200234

235+
export type ProfileEncodingResult =
236+
| {
237+
type: 'SUCCESS';
238+
profileData: Blob;
239+
}
240+
| { type: 'ERROR'; error: Error };
241+
242+
function unwrapEncodedProfile(encodingResult: ProfileEncodingResult): Blob {
243+
if (encodingResult.type === 'ERROR') {
244+
throw encodingResult.error;
245+
}
246+
return encodingResult.profileData;
247+
}
248+
249+
export type InflightProfileEncoding = {
250+
sanitizedProfile: Profile;
251+
encodingPromise: Promise<ProfileEncodingResult>;
252+
};
253+
254+
/**
255+
* Kick off "encoding" of the sanitized profile. Specifically this means:
256+
* - Compute the sanitized profile
257+
* - Serialize the profile to a buffer
258+
* - Kick off the asynchronous compression of the buffer
259+
*
260+
* The asynchronous compression can take a few seconds, so we want to kick
261+
* it off immediately when the profile publishing panel is opened. We also
262+
* want to be able to make use of the current in-flight compression if the
263+
* user clicks the upload button before compression is done. This is why
264+
* we return an `InflightProfileEncoding` object from this action; it contains
265+
* a promise which lets other parts of the publishing pipeline wait on the
266+
* compressed results.
267+
*
268+
* This thunk action is synchronous.
269+
*/
270+
export function encodeSanitizedProfile(
271+
previousInflightEncoding?: InflightProfileEncoding
272+
): ThunkAction<InflightProfileEncoding> {
273+
return (dispatch, getState): InflightProfileEncoding => {
274+
const state = getState();
275+
const sanitizedProfile = getSanitizedProfile(state).profile;
276+
277+
if (previousInflightEncoding?.sanitizedProfile === sanitizedProfile) {
278+
// No need to kick of another compression. The current encoding may still
279+
// be in-flight, and returning the original promise allows the caller to
280+
// await it.
281+
return previousInflightEncoding;
282+
}
283+
284+
const encodingState = getSanitizedProfileEncodingState(state);
285+
if (
286+
encodingState.phase === 'DONE' &&
287+
encodingState.sanitizedProfile === sanitizedProfile
288+
) {
289+
// We already have an encoded version of this profile in our state! Use it.
290+
return {
291+
sanitizedProfile,
292+
encodingPromise: Promise.resolve({
293+
type: 'SUCCESS',
294+
profileData: encodingState.profileData,
295+
}),
296+
};
297+
}
298+
299+
// Kick off a new encoding for this profile. Don't await the promise,
300+
// just return it as part of the InflightProfileEncoding.
301+
const encodingPromise: Promise<ProfileEncodingResult> = (async function () {
302+
try {
303+
dispatch(sanitizedProfileEncodingStarted(sanitizedProfile));
304+
const gzipData = await compress(serializeProfile(sanitizedProfile));
305+
const blob = new Blob([gzipData], { type: 'application/octet-binary' });
306+
dispatch(sanitizedProfileEncodingCompleted(sanitizedProfile, blob));
307+
return { type: 'SUCCESS', profileData: blob };
308+
} catch (error) {
309+
dispatch(sanitizedProfileEncodingFailed(sanitizedProfile, error));
310+
console.error('Error while compressing the profile data', error);
311+
return { type: 'ERROR', error };
312+
}
313+
})();
314+
315+
return { sanitizedProfile, encodingPromise };
316+
};
317+
}
318+
201319
/**
202320
* This function starts the profile sharing process. Takes an optional argument that
203321
* indicates if the share attempt is being made for the second time. We have two share
@@ -210,7 +328,9 @@ async function persistJustUploadedProfileInformationToDb(
210328
* The return value is used for tests to determine if the request went all the way
211329
* through (true) or was quit early due to the generation value being invalidated (false).
212330
*/
213-
export function attemptToPublish(): ThunkAction<Promise<boolean>> {
331+
export function attemptToPublish(
332+
previousInflightEncoding?: InflightProfileEncoding
333+
): ThunkAction<Promise<boolean>> {
214334
return async (dispatch, getState) => {
215335
try {
216336
sendAnalytics({
@@ -244,21 +364,29 @@ export function attemptToPublish(): ThunkAction<Promise<boolean>> {
244364
dispatch(uploadCompressionStarted(abortfunction));
245365

246366
const sanitizedInformation = getSanitizedProfile(prePublishedState);
247-
const gzipData = await getSanitizedProfileData(prePublishedState);
367+
const profileEncoding = dispatch(
368+
encodeSanitizedProfile(previousInflightEncoding)
369+
);
370+
const encodingResult = await profileEncoding.encodingPromise;
248371

249372
// The previous line was async, check to make sure that this request is still valid.
250373
// The upload could have been aborted while we were compressing the data.
251374
if (uploadGeneration !== getUploadGeneration(getState())) {
252375
return false;
253376
}
254377

378+
const encodedProfile = unwrapEncodedProfile(encodingResult);
379+
255380
dispatch(uploadStarted());
256381

257382
// Upload the profile, and notify it with the amount of data that has been
258383
// uploaded.
259-
const hashOrToken = await startUpload(gzipData, (uploadProgress) => {
260-
dispatch(updateUploadProgress(uploadProgress));
261-
});
384+
const hashOrToken = await startUpload(
385+
encodedProfile,
386+
(uploadProgress) => {
387+
dispatch(updateUploadProgress(uploadProgress));
388+
}
389+
);
262390

263391
const hash = extractProfileTokenFromJwt(hashOrToken);
264392

0 commit comments

Comments
 (0)