Skip to content

Optimize Dangerfile for better performance #35010

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed

Optimize Dangerfile for better performance #35010

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
182 changes: 100 additions & 82 deletions dangerfile.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,7 @@ const {markdown, danger, warn} = require('danger');
const {promisify} = require('util');
const glob = promisify(require('glob'));
const gzipSize = require('gzip-size');
const {writeFileSync} = require('fs');

const {readFileSync, statSync} = require('fs');
const {writeFileSync, readFileSync, statSync, existsSync} = require('fs');

const BASE_DIR = 'base-build';
const HEAD_DIR = 'build';
Expand Down Expand Up @@ -100,6 +98,66 @@ function row(result, baseSha, headSha) {
return rowArr.join(' | ');
}

function getArtifactStats(dir, artifactPath) {
const fullPath = `${dir}/${artifactPath}`;
if (!existsSync(fullPath)) {
return null;
}
return {
size: statSync(fullPath).size,
gzipSize: gzipSize.fileSync(fullPath),
};
}

function createResult(artifactPath, baseStats, headStats) {
const baseSize = baseStats?.size ?? 0;
const baseSizeGzip = baseStats?.gzipSize ?? 0;
const headSize = headStats?.size ?? 0;
const headSizeGzip = headStats?.gzipSize ?? 0;

let change, changeGzip;
if (!baseStats && headStats) {
// New file
change = Infinity;
changeGzip = Infinity;
} else if (baseStats && !headStats) {
// Deleted file
change = -1;
changeGzip = -1;
} else {
// Modified file
change = baseSize === 0 ? 0 : (headSize - baseSize) / baseSize;
changeGzip =
baseSizeGzip === 0 ? 0 : (headSizeGzip - baseSizeGzip) / baseSizeGzip;
}

return {
path: artifactPath,
headSize,
headSizeGzip,
baseSize,
baseSizeGzip,
change,
changeGzip,
};
}

function isSignificantChange(result) {
return (
Math.abs(result.change) > SIGNIFICANCE_THRESHOLD ||
result.change === Infinity ||
result.change === -1
);
}

function isCriticalChange(result) {
return (
Math.abs(result.change) > CRITICAL_THRESHOLD ||
result.change === Infinity ||
result.change === -1
);
}

(async function () {
// Use git locally to grab the commit which represents the place
// where the branches differ
Expand Down Expand Up @@ -132,72 +190,40 @@ function row(result, baseSha, headSha) {
];
if (
commitFiles.every(filename => filename.includes('packages/react-devtools'))
)
) {
return;
}

const resultsMap = new Map();

// Find all the head (current) artifacts paths.
const headArtifactPaths = await glob('**/*.js', {cwd: 'build'});
for (const artifactPath of headArtifactPaths) {
try {
// This will throw if there's no matching base artifact
const baseSize = statSync(BASE_DIR + '/' + artifactPath).size;
const baseSizeGzip = gzipSize.fileSync(BASE_DIR + '/' + artifactPath);

const headSize = statSync(HEAD_DIR + '/' + artifactPath).size;
const headSizeGzip = gzipSize.fileSync(HEAD_DIR + '/' + artifactPath);
resultsMap.set(artifactPath, {
path: artifactPath,
headSize,
headSizeGzip,
baseSize,
baseSizeGzip,
change: (headSize - baseSize) / baseSize,
changeGzip: (headSizeGzip - baseSizeGzip) / baseSizeGzip,
});
} catch {
// There's no matching base artifact. This is a new file.
const baseSize = 0;
const baseSizeGzip = 0;
const headSize = statSync(HEAD_DIR + '/' + artifactPath).size;
const headSizeGzip = gzipSize.fileSync(HEAD_DIR + '/' + artifactPath);
resultsMap.set(artifactPath, {
path: artifactPath,
headSize,
headSizeGzip,
baseSize,
baseSizeGzip,
change: Infinity,
changeGzip: Infinity,
});
}
}

// Check for base artifacts that were deleted in the head.
const baseArtifactPaths = await glob('**/*.js', {cwd: 'base-build'});
for (const artifactPath of baseArtifactPaths) {
if (!resultsMap.has(artifactPath)) {
const baseSize = statSync(BASE_DIR + '/' + artifactPath).size;
const baseSizeGzip = gzipSize.fileSync(BASE_DIR + '/' + artifactPath);
const headSize = 0;
const headSizeGzip = 0;
resultsMap.set(artifactPath, {
path: artifactPath,
headSize,
headSizeGzip,
baseSize,
baseSizeGzip,
change: -1,
changeGzip: -1,
});
}
// Find all artifact paths from both base and head builds
const [headArtifactPaths, baseArtifactPaths] = await Promise.all([
glob('**/*.js', {cwd: HEAD_DIR}),
glob('**/*.js', {cwd: BASE_DIR}),
]);

// Create a set of all unique artifact paths
const allArtifactPaths = new Set([
...headArtifactPaths,
...baseArtifactPaths,
]);

// Process all artifacts in a single pass
for (const artifactPath of allArtifactPaths) {
const baseStats = getArtifactStats(BASE_DIR, artifactPath);
const headStats = getArtifactStats(HEAD_DIR, artifactPath);

const result = createResult(artifactPath, baseStats, headStats);
resultsMap.set(artifactPath, result);
}

const results = Array.from(resultsMap.values());
results.sort((a, b) => b.change - a.change);

let criticalResults = [];
const criticalResults = [];
const significantResults = [];

// Process critical artifacts first (in fixed order)
for (const artifactPath of CRITICAL_ARTIFACT_PATHS) {
const result = resultsMap.get(artifactPath);
if (result === undefined) {
Expand All @@ -208,33 +234,25 @@ function row(result, baseSha, headSha) {
);
}
criticalResults.push(row(result, baseSha, headSha));

// Also add to significant results if it meets the threshold
if (isSignificantChange(result)) {
significantResults.push(row(result, baseSha, headSha));
}
}

let significantResults = [];
// Process remaining results
for (const result of results) {
// If result exceeds critical threshold, add to top section.
if (
(result.change > CRITICAL_THRESHOLD ||
0 - result.change > CRITICAL_THRESHOLD ||
// New file
result.change === Infinity ||
// Deleted file
result.change === -1) &&
// Skip critical artifacts. We added those earlier, in a fixed order.
!CRITICAL_ARTIFACT_PATHS.has(result.path)
) {
// Skip critical artifacts as they were already processed
if (CRITICAL_ARTIFACT_PATHS.has(result.path)) {
continue;
}

if (isCriticalChange(result)) {
criticalResults.push(row(result, baseSha, headSha));
}

// Do the same for results that exceed the significant threshold. These
// will go into the bottom, collapsed section. Intentionally including
// critical artifacts in this section, too.
if (
result.change > SIGNIFICANCE_THRESHOLD ||
0 - result.change > SIGNIFICANCE_THRESHOLD ||
result.change === Infinity ||
result.change === -1
) {
if (isSignificantChange(result)) {
significantResults.push(row(result, baseSha, headSha));
}
}
Expand Down Expand Up @@ -279,4 +297,4 @@ ${significantResults.join('\n')}
} else {
markdown(message);
}
})();
})();