fix(ci): optimize workflows, fix build ordering, and add caching#24611
fix(ci): optimize workflows, fix build ordering, and add caching#24611hiSandog wants to merge 1 commit intogoogle-gemini:mainfrom
Conversation
- Fix critical npm ci/build ordering bug in ci.yml (Linux + Mac) - Add TypeScript build cache to ci.yml, chained_e2e.yml, deflake.yml - Add npm cache to E2E workflows (was missing entirely) - Remove wasteful npm cache clean --force on macOS E2E jobs - Increase vitest maxThreads 4->8 for core and cli packages - Add LRU eviction (maxCapacity) to CacheService Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request focuses on optimizing the continuous integration pipeline and enhancing the internal caching utility. By correcting build sequences, introducing comprehensive caching strategies, and tuning test parallelism, the changes aim to significantly reduce build times and resource consumption. Additionally, the introduction of an LRU eviction policy for the CacheService ensures better memory management in high-throughput environments. Highlights
Ignored Files
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request implements a Least Recently Used (LRU) eviction strategy for the CacheService in packages/core by introducing a maxCapacity option and tracking access order for Map-based storage. Additionally, the Vitest configuration for packages/cli and packages/core has been updated to increase maxThreads from 4 to 8. I have no feedback to provide.
Summary
This PR addresses several issues in the CI workflows that cause unnecessary build times and includes a correctness fix for a critical build ordering bug.
Changes
1. Critical: Fix
npm ci/npm run buildordering in ci.ymlOn Linux and Mac CI jobs,
npm run buildwas running beforenpm ci. This means builds were using potentially stale cached artifacts from previous workflow runs instead of freshly installed dependencies. Fixed by movingnpm cito run beforenpm run build.2. Add TypeScript build caching to all major workflows
Added
actions/cache@v4for TypeScript build outputs (packages/*/dist,packages/*/*.tsbuildinfo) to:ci.yml(Linux + Mac)chained_e2e.yml(Linux + Mac + Evals)deflake.yml(Linux + Mac)This avoids redundant
tsc --buildrecompilation across workflow runs that use the same commit SHA.3. Add npm dependency caching to E2E workflows
Added
cache: 'npm'to setup-node inchained_e2e.ymlanddeflake.yml, which were missing npm cache configuration entirely.4. Remove
npm cache clean --forceon macOS E2E jobsBoth
chained_e2e.ymlanddeflake.ymlhad a step that forcibly destroyed the npm cache on macOS runners. Removing this allows subsequent workflow runs to benefit from npm dependency caching.5. Increase vitest thread limits for large packages
Increased
maxThreadsfrom 4 to 8 for:packages/core/vitest.config.ts(909 test files)packages/cli/vitest.config.ts(233 test files)The 4-thread limit was unnecessarily constraining test parallelism on 16-core CI runners.
6. Add LRU eviction to CacheService
Added
maxCapacityoption toCacheOptionsinterface inpackages/core/src/utils/cache.ts. When the cache exceeds the configured capacity, the least recently used entry is evicted. This prevents unbounded memory growth in long-running or high-throughput scenarios.