From 0c075001832c937940009c105dca4de17fe612dc Mon Sep 17 00:00:00 2001 From: Kevin Gozali Date: Mon, 18 Oct 2021 11:04:22 -0700 Subject: [PATCH] OSS CI: Store bundle size info for release branch as well (#32418) Summary: Pull Request resolved: https://github.com/facebook/react-native/pull/32418 This commit does 2 things: * Process and store stats for *-stable branches, in addition to main * Print out the new stats to stdout, so that CI jobs can display them for verification purpose This also means a new field `branch` is used for the Firestore data. Changelog: [Internal] Reviewed By: hramos Differential Revision: D31717251 fbshipit-source-id: 9dbfa8fb8f0243c013dcd822230400d26c09eaa4 --- bots/datastore.js | 8 ++- bots/report-bundle-size.js | 115 +++++++++++++++++++++---------------- 2 files changed, 73 insertions(+), 50 deletions(-) diff --git a/bots/datastore.js b/bots/datastore.js index ca04ca66d408df..04fbd69acc4c36 100644 --- a/bots/datastore.js +++ b/bots/datastore.js @@ -92,12 +92,14 @@ function getBinarySizesCollection(db) { * @param {firebase.firestore.CollectionReference} collection * @param {string} sha The Git SHA used to identify the entry * @param {firebase.firestore.UpdateData} data The data to be inserted/updated + * @param {string} branch The Git branch where this data was computed for * @returns {Promise} */ -function createOrUpdateDocument(collectionRef, sha, data) { +function createOrUpdateDocument(collectionRef, sha, data, branch) { const stampedData = { ...data, timestamp: firestore.Timestamp.now(), + branch, }; const docRef = firestore.doc(collectionRef, sha); return firestore.updateDoc(docRef, stampedData).catch(async error => { @@ -115,14 +117,16 @@ function createOrUpdateDocument(collectionRef, sha, data) { * Returns the latest document in collection. * * @param {firebase.firestore.CollectionReference} collection + * @param {string} branch The Git branch for the data * @returns {Promise} */ -async function getLatestDocument(collectionRef) { +async function getLatestDocument(collectionRef, branch) { try { const querySnapshot = await firestore.getDocs( firestore.query( collectionRef, firestore.orderBy('timestamp', 'desc'), + firestore.where('branch', '==', branch), firestore.limit(1), ), ); diff --git a/bots/report-bundle-size.js b/bots/report-bundle-size.js index 75cc461c150f5d..fc9161688f8bac 100644 --- a/bots/report-bundle-size.js +++ b/bots/report-bundle-size.js @@ -25,7 +25,7 @@ const datastore = require('./datastore'); const {createOrUpdateComment} = require('./make-comment'); /** - * Generates and submits a comment. If this is run on main branch, data is + * Generates and submits a comment. If this is run on the main or release branch, data is * committed to the store instead. * @param {{ 'android-hermes-arm64-v8a'?: number; @@ -47,8 +47,7 @@ async function reportSizeStats(stats, replacePattern) { ); const collection = datastore.getBinarySizesCollection(store); - // Collect the current sizes for main branch only. - if (GITHUB_REF === 'main') { + if (GITHUB_REF === 'main' || GITHUB_REF.endsWith('-stable')) { // Ensure we only store numbers greater than zero. const validatedStats = Object.keys(stats).reduce((validated, key) => { const value = stats[key]; @@ -59,65 +58,85 @@ async function reportSizeStats(stats, replacePattern) { validated[key] = value; return validated; }, {}); + if (Object.keys(validatedStats).length > 0) { + // Print out the new stats + const document = + (await datastore.getLatestDocument(collection, GITHUB_REF)) || {}; + const formattedStats = formatBundleStats(document, validatedStats); + console.log(formattedStats); + await datastore.createOrUpdateDocument( collection, GITHUB_SHA, validatedStats, + GITHUB_REF, ); } - } else if (GITHUB_REF.endsWith('-stable')) { - console.log(`Skipping bundle size reporting for branch: ${GITHUB_REF}`); } else { - const document = await datastore.getLatestDocument(collection); - - const diffFormatter = new Intl.NumberFormat('en', {signDisplay: 'always'}); - const sizeFormatter = new Intl.NumberFormat('en', {}); - - // | Platform | Engine | Arch | Size (bytes) | Diff | - // |:---------|:-------|:------------|-------------:|-----:| - // | android | hermes | arm64-v8a | 9437184 | ±0 | - // | android | hermes | armeabi-v7a | 9015296 | ±0 | - // | android | hermes | x86 | 9498624 | ±0 | - // | android | hermes | x86_64 | 9965568 | ±0 | - // | android | jsc | arm64-v8a | 9236480 | ±0 | - // | android | jsc | armeabi-v7a | 8814592 | ±0 | - // | android | jsc | x86 | 9297920 | ±0 | - // | android | jsc | x86_64 | 9764864 | ±0 | - // | android | jsc | x86_64 | 9764864 | ±0 | - // | ios | - | universal | 10715136 | ±0 | - const comment = [ - '| Platform | Engine | Arch | Size (bytes) | Diff |', - '|:---------|:-------|:-----|-------------:|-----:|', - ...Object.keys(stats).map(identifier => { - const [size, diff] = (() => { - const statSize = stats[identifier]; - if (!statSize) { - return ['n/a', '--']; - } else if (!(identifier in document)) { - return [statSize, 'n/a']; - } else { - return [ - sizeFormatter.format(statSize), - diffFormatter.format(statSize - document[identifier]), - ]; - } - })(); - - const [platform, engineOrArch, ...archParts] = identifier.split('-'); - const arch = archParts.join('-') || engineOrArch; - const engine = arch === engineOrArch ? '-' : engineOrArch; // e.g. 'ios-universal' - return `| ${platform} | ${engine} | ${arch} | ${size} | ${diff} |`; - }), - '', - `Base commit: ${document.commit}`, - ].join('\n'); + // For PRs, always compare vs main. + const document = + (await datastore.getLatestDocument(collection, 'main')) || {}; + const comment = formatBundleStats(document, stats); createOrUpdateComment(comment, replacePattern); } await datastore.terminateStore(store); } +/** + * Format the new bundle stats as compared to the latest stored entry. + * @param {firebase.firestore.DocumentData} document the latest entry to compare against + * @param {firebase.firestore.UpdateData} stats The stats to be formatted + * @returns {string} + */ +function formatBundleStats(document, stats) { + const diffFormatter = new Intl.NumberFormat('en', {signDisplay: 'always'}); + const sizeFormatter = new Intl.NumberFormat('en', {}); + + // | Platform | Engine | Arch | Size (bytes) | Diff | + // |:---------|:-------|:------------|-------------:|-----:| + // | android | hermes | arm64-v8a | 9437184 | ±0 | + // | android | hermes | armeabi-v7a | 9015296 | ±0 | + // | android | hermes | x86 | 9498624 | ±0 | + // | android | hermes | x86_64 | 9965568 | ±0 | + // | android | jsc | arm64-v8a | 9236480 | ±0 | + // | android | jsc | armeabi-v7a | 8814592 | ±0 | + // | android | jsc | x86 | 9297920 | ±0 | + // | android | jsc | x86_64 | 9764864 | ±0 | + // | android | jsc | x86_64 | 9764864 | ±0 | + // | ios | - | universal | 10715136 | ±0 | + const formatted = [ + '| Platform | Engine | Arch | Size (bytes) | Diff |', + '|:---------|:-------|:-----|-------------:|-----:|', + ...Object.keys(stats).map(identifier => { + const [size, diff] = (() => { + const statSize = stats[identifier]; + if (!statSize) { + return ['n/a', '--']; + } else if (!(identifier in document)) { + return [statSize, 'n/a']; + } else { + return [ + sizeFormatter.format(statSize), + diffFormatter.format(statSize - document[identifier]), + ]; + } + })(); + + const [platform, engineOrArch, ...archParts] = identifier.split('-'); + const arch = archParts.join('-') || engineOrArch; + const engine = arch === engineOrArch ? '-' : engineOrArch; // e.g. 'ios-universal' + return `| ${platform} | ${engine} | ${arch} | ${size} | ${diff} |`; + }), + '', + `Base commit: ${document.commit || ''}`, + `Branch: ${document.branch || ''}`, + ].join('\n'); + + return formatted; +} + /** * Returns the size of the file at specified path in bytes. * @param {fs.PathLike} path