-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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.
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
Epic: road to faster CI/CD #27359
Labels
Area: Build System
CI
Resolution: Soft Close
Soft closing inactive issues over a certain period
Type: Epic
Comments
This was referenced Apr 18, 2023
feat(react-conformance): add new TS config api to be able to specify configName and configDir
#27664
Merged
1 task
17 tasks
keep open |
This issue has not had activity for over 180 days! We're adding Soft close label and will close it soon for house-keeping purposes. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
Area: Build System
CI
Resolution: Soft Close
Soft closing inactive issues over a certain period
Type: Epic
Last couple of days v-build team was concluding analysis of our repo tools/CI etc. Based on this analysis we identified major bottlenecks within our tools/infra and experimented with mitigating those.
This issues is a summarized document of all the data and findings and should be taken as source of truth for actionables going forward with one goal - making our CI fast/er and reliable 🚅
CI data (time metrics from running pipelines on whole monorepo)
Legend:
isConformance
/ https://uifabric.visualstudio.com/fabricpublic/_build/results?buildId=291729&view=logs&j=4fecd60f-6595-5d51-257b-4743b034536f&t=0df5d56b-1f79-5ca2-2e20-703f84ecb970Optimizations
During analysis of our tasks/pipelines we found various opportunities how to improve our pipeline speeds to enable faster time do delivery.
Following Chapters contain most impactful findings with some amount of detail for how to enable them.
We were able to implement some of these suggestions during experimentation phase which gave us following worst case scenario pipeline durations.
Stop using TS path aliases on CI
Implementation:
Using TS path aliases provides excellent DX and blazing fast Application Bundling speeds with tools like swc or esbuild.
For running
type-check
and d.ts emit within our codebase it inflicts huge performance penalty, as TSC needs to traverse and parse all the paths on every run.For
type-check
in particular, which is used by runningtsc -b
on ts solution configuration per project, the performance penalty is the biggest.By stopping using path aliases on CI for type-checks we will get 40% speed bumps for type-checking !
Speed metrics:
Blockers:
Storybook Stories
Because our approach to stories we create circular dependency between packages.
Example:
react-table
stories import fromreact-data-grid-react-window
, whilereact-data-grid-react-window
production code imports fromreact-table
.This makes it impossible to implement turning off path aliases for type-checks on CI, because following task relationship will not work:
generate-api
-->type-check
Possible solutions (Storybook Stories)
1. lets keep things as is - having the per penalty increased with every new line of code
2. make stories separate packages
3. make stories separate packages - via nx "hack"
This approach might be a ticking bomb as we are approaching a package within package (regarding resolution algorithms and tools).
Clean approach would be to restructure folder structure to following:
4. generate dts manually before running
type-check
on CIBefore
type-check
pipeline is run we manually generate type declaration for all v9 packages present in stories.Simplified flow:
tsc -p tsconfig.lib.json
withinreact-components
tsc -p tsconfig.lib.json
on those dependencies that exist in stories but are not part ofreact-components
suitetsconfig.base.dts.json
that will map to.d.ts
path aliases instead of.ts
source filestype-check
pipelineEnable
incremental
for TS solution configs +emitDeclarationOnly
By enabling
incremental:true
for projects that use TS Solution configs, we will get around 15% perf boost.This improvement will be leveraged on [
type-check
--depends on-->generate-api
] task execution relationship,where
type-check
will leverage incremental emit metadata from runningtsc -p tsconfig.lib.json (part of generate api)
.Speed metrics:
Migrate react(v8) to TS solution configs
Besides small speed benefits, using solution configs will provide additional ones like:
Speed metrics:
@fluentui/react test --no-cache --runInBand
Emit declarations only - react-northstar
Implementation:
v0 uses solely babel for transpilation thus tsc is doing unnecessary work. TSC should generate only declaration files. This will give use approx 10% speed bump.
Jest Test transform tweaks
Implementation:
test: use
isolatedModules
for all ts-jest configs to lower memory footprint on CI #27670use
@swc/jest
for v9 - this will give us up to 10% speed boostuse ts-jest with
isolatedModules
for v8Speed metrics:
@fluentui/react test --no-cache --runInBand
isolatedModules:true
stop running Code Coverage on CI
Implementation:
Slows test execution up to 10% (present in react-northstar)
gulp test --config ./jest.config.js --coverage --maxWorkers=2 --no-cache
gulp test --config ./jest.config.js --maxWorkers=2
Tests memory leaks
Implementation:
Both v0 and v8 tests contain memory leaks and consume above 2GB of memory per test !!!
By cleaning up those tests to not leak / not drastically add memory heap, we could make them run faster as is and enable full
maxWorkers
count (we use 8 core CPUs on CI) which would drastically cut time execution.Speed metrics:
Components Conformance Tests
Implementation:
Speed metrics::
Legend:
ts.createProgram()
.yarn workspace @fluentui/react-text --no-cache --runInBand
Summary:
From the performance metrics 4th and 5th approach is clear winner in terms of performance.
🥶 Unfortunately, that approach (nor 4 nor 5) cannot be possible achieved with current conformance architecture.
Why?
Because of jest architecture - every test is executed in separate sandbox and executed in parallel( runInBand wont solve anything), thus we cannot cache anything per test scenario (in our case TS program instance) on global level (only serializable data via globalConfig hook).
What's the issue with TS program ? it is re-created on every test for exactly same set of compiler options and files. This has also implication where "bigger codebase of package" === "longer time to execute"
narrow down TS program
For v9 we use TS path aliases. react-conformance will consume those solution config which cause to load huge trees of programs that inflict big perf slowdown.
We can consume
tsconfig.lib.json
for ts solution projects which will gain us 45% speed boostmitigate need of TS program use
We can do partial rewrite of react-conformance to create TS program via
removing creation of TS Program via ``ts.createProgram()
only when necessary (lazily). With that we can get up to 70% speed boost case ( depends on usage of conformance test scenarios )architecture rewrite
Our UI controls conformance architecture inflicts one of the the biggest performance impacts (bottleneck). It needs to be rewritten in a way where TS program are created only once - a set of Eslint rules is excellent candidate for this.
Task runner/s change
Replace gulp and just with nx executors/native node scripts.
Every gulp and just execution takes 2-3 seconds.
Example:
running
yarn workspace @fluentui/react-button build
:e2e
target leveraging TS path aliases for bundlingATM e2e task execution is mixed with bundle and deploy in 1 job. We have setup already in place where we don't need to physically trigger
build
norbundle
tasks fore2e
execution, thus decouple these jobs.By doing that we can get following speed improvements:
nx run-many --target=e2e --skip-cache
Solution Suggestion:
Deploy & E2E
pipelineapps
build
target changemost of application that live within
apps
leverage path aliases with esbuild/swc.Using that setup for bundling is more than 50% faster then using incremental build based on our repo dependency tree.
Solution Suggestion:
We could rename target
build
tobundle-app
. This would stop triggeringbuild
on every package on every PR/Release.Decoupling dependency tree
Mitigate v8 dependency tree creep which triggers literally all v8 packages "re-builds" on any change (v9)
Enable caching/distributed caching
we execute
build
task multiple times on every separate pipeline. This adds around 4 minutes to every pipeline we ran.Solution Suggestion:
Cache node_modules (yarn install)
yarn install
takes significant amount for every pipeline job (up to 1m 30s).Solution Suggestion:
This is a low hanging fruit that can be improved by caching node_modules for
yarn install
(from 2 minutes to 5 seconds)Improve master pipeline
On every PR merge (push to master), we run pipelines on default branch (master), which triggers
lint,build,test,type-check
on whole monorepo.metrics of this approach:
Solution Suggestion:
There is no need real need to run everything on every push to master.
We should run pipelines only against latest successful merge commit.
yarn nx affected --targets=build,lint,type-check,test --base=LAST_SUCCESSFUL_SHA --head=HEAD
Actionable
The text was updated successfully, but these errors were encountered: