Skip to content

Improves CI performance - #22995

Draft
NickSdot wants to merge 8 commits into
php:masterfrom
NickSdot:perf/ci-build-times
Draft

Improves CI performance#22995
NickSdot wants to merge 8 commits into
php:masterfrom
NickSdot:perf/ci-build-times

Conversation

@NickSdot

@NickSdotNickSdot commented Aug 2, 2026

Copy link
Copy Markdown
Contributor

tl/dr: ~45m -> ~22m; this is CI-only stuff -- everything in #22917 reduces further.

Results

f47a44e (perf: speed up Windows CI)

Windows x64BeforeAfterImprovement
Setup + build10:533:2269%
Tests30:3714:4852%
Full job41:5518:4855%

Cache hit rate: ~95%.

8e526f1 (perf: speed up benchmark checkout)

Avoids downloading unnecessary file contents. From ~1:11 to ~0:20, which is a 72% improvement.

653fa83 (perf: keep non-Windows compiler caches fresh)

Before: the older the weekly cache, the lower the hit rate. 2026-07-27 was ~99% (0:30s) while 2026-08-02 was only ~25% (6:52). Rolling cache solves that.

Compile jobBefore (1)AfterImprovement
Linux release7:540:2894%
Benchmark2:330:1789%
Unit tests2:060:1489%
Linux ASAN1:500:2775%
macOS4:071:3064%
Alpine ASAN2:370:5764%
Linux x322:100:5756%

Cache hit rate: ~99%.
(1) when cache not fresh.

6c933cb (perf: skip unused ASAN setup)

SetupBeforeAfterImprovement
Service containers1:240:1385%
MSSQL setup0:14Skipped100%
Apt dependencies0:540:4222%
Total targeted setup2:320:5564%

Saves 1:37 per ASAN run. Depending on download speeds shaves off 1–3 min.

a33ae4c (perf: run Function JIT tests concurrently)

Splits from sequentially to separate jobs for normal (16:37m) and JIT (18:47 ); results in 11 minutes earlier finish.

3dcc018 (perf: shard long running jobs)

Critical path dropped from 44:27 to 22:49 (49%). Slowest shards finished in 19:03 for Linux ASAN, 17:39 for Windows x64 ZTS, and 14:43 for Alpine ASAN.

58c43d5 (perf: aded circle ci compile caching)

CircleCIBeforeWarm cacheImprovement
Compile8:173:2359%

Cache hit rate: ~99%

Warm cache compile down from ~8:17 to ~3:23, which is 59% improvement. Works slightly different than the other caching. Circle isolates caches that belong to external forks. Trusted master builds roll the source repository cache forward; same-repository PRs can restore it, fork PRs start cold and then roll their own forward.

@NickSdot
NickSdotforce-pushed the perf/ci-build-times branch 2 times, most recently from c4f48b7 to 6ce446cCompareAugust 2, 2026 09:16
@NickSdot
NickSdotforce-pushed the perf/ci-build-times branch 2 times, most recently from 87c7e25 to 541ac21CompareAugust 2, 2026 10:01
@NickSdot
NickSdotforce-pushed the perf/ci-build-times branch 10 times, most recently from 4010558 to b9e37cfCompareAugust 2, 2026 17:09
@NickSdot
NickSdotforce-pushed the perf/ci-build-times branch 3 times, most recently from 5f23b04 to 70b3becCompareAugust 2, 2026 19:41
Comment thread.github/matrix.php Outdated
@NickSdot
NickSdotforce-pushed the perf/ci-build-times branch from 70b3bec to 9de2a2eCompareAugust 2, 2026 21:18
@NickSdotNickSdot changed the title Improves CI build performanceImproves CI performanceAug 3, 2026
@NickSdot
NickSdotforce-pushed the perf/ci-build-times branch from 6a1ba96 to 2fb2a6eCompareAugust 3, 2026 09:17
@NickSdot
NickSdotforce-pushed the perf/ci-build-times branch from 2fb2a6e to 09834c1CompareAugust 3, 2026 09:32
@NickSdot
NickSdotforce-pushed the perf/ci-build-times branch from 6ac7397 to ae7dcecCompareAugust 3, 2026 15:06
@NickSdot
NickSdotforce-pushed the perf/ci-build-times branch from ae7dcec to 1c8ede1CompareAugust 3, 2026 15:42
@NickSdot

Copy link
Copy Markdown
ContributorAuthor

@arnaud-lb maybe it is worth prioritising this one before the others. Would give me a much faster feedback loop for all the rest that likely will receive some more pushes if there is feedback from you to address. Will you look at this one too or should I mark as ready to ping the reviewers?

@NickSdot
NickSdot marked this pull request as ready for review August 4, 2026 15:32
@NickSdotNickSdot mentioned this pull request Aug 5, 2026

@arnaud-lbarnaud-lb left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! Could you split this PR into multiple ones? Each commit seems pretty independent and could be a separate PR.

Comment on lines +1 to +15
!if "$(PARALLEL_BUILD)" == "yes"
ext\json\php_json_scanner_defs.h: ext\json\json_scanner.c
ext\json\json_scanner.c: ext\json\json_scanner.re ext\json\json_parser.tab.h
$(RE2C) $(RE2C_FLAGS) -t ext/json/php_json_scanner_defs.h -bci -o ext/json/json_scanner.c ext/json/json_scanner.re

ext\json\json_parser.tab.h: ext\json\json_parser.tab.c
ext\json\json_parser.tab.c: ext\json\json_parser.y
$(BISON) $(BISON_FLAGS) --defines -l ext/json/json_parser.y -o ext/json/json_parser.tab.c
!else
ext\json\json_scanner.c ext\json\php_json_scanner_defs.h: ext\json\json_scanner.re ext\json\json_parser.tab.h
$(RE2C) $(RE2C_FLAGS) -t ext/json/php_json_scanner_defs.h -bci -o ext/json/json_scanner.c ext/json/json_scanner.re

ext\json\json_parser.tab.c ext\json\json_parser.tab.h: ext\json\json_parser.y
$(BISON) $(BISON_FLAGS) --defines -l ext/json/json_parser.y -o ext/json/json_parser.tab.c
!endif

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's unfortunate that we have to maintain two sets of rules for jom/nmake in some cases. Would it be possible to write a set of rules that works for both? Using a stamp file might work:

ext\json\json_scanner.c ext\json\php_json_scanner_defs.h: ext\json\json_scanner.stamp
ext\json\json_scanner.stamp: ext\json\json_scanner.re ext\json\json_parser.tab.h
$(RE2C) $(RE2C_FLAGS) -t ext/json/php_json_scanner_defs.h -bci -o ext/json/json_scanner.c ext/json/json_scanner.re
echo done > ext\json\json_scanner.stamp

if "%CLANG_TOOLSET%" equ "1" goto build_clang

sccache --zero-stats
jom /NOLOGO CC="sccache cl.exe"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! I didn't know jom, but it seems to be a drop-in replacement for nmake with parallel execution support. There is at least one previous effort to parallelize windows builds but it was abandoned. Linking it here for reference: GH-17445. I see that you address some of the issues mentioned in the other issue.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wasn't aware. Is this just for reference or is there anything specific I should look at in the linked PR?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mostly for reference so that we can find related work from either of these PRs, but also to point at #17445 (comment) which mentions some issues that may be relevant here too. I believe that you've addressed at least the PDB one.

@NickSdot

Copy link
Copy Markdown
ContributorAuthor

Thanks for prioritising reviewing this one!

Nice! Could you split this PR into multiple ones? Each commit seems pretty independent and could be a separate PR.

Did you see how many PRs I have open? I am loosing overview, and would kindly ask to not split again here. 😅

All commits together solve one problem. They depend on each other to get the total CI run time balanced. Each commit is reviewable on it's own easily. Please let's not split this just for the sake of it. 🙏

@iluuu1994

Copy link
Copy Markdown
Member

IMO, there are some useful things in this PR, and some less useful things.

  • .github/actions/apt-x64/action.yml is good.
  • filter: blob:none is good.
  • .github/actions/ccache/action.yml: The current solution exists specifically because too many cache entries were pushed, see [CI] Fix ineffective ccache #21258.
  • Shards: I dislike the idea of splitting jobs into multiple parts if the entire build step is repeated. In GitLab CI, we used to have a build step and could then copy artifacts to the next stages that could trivially be parallelized. GitHub doesn't work this way, and repeating the build 3 times defeats the purpose. We don't just want shorter parallel builds, we also want less resource usage.
  • Windows: Can't comment much on the Windows stuff, maybe @shivammathur can have a look for those.

@staabm

Copy link
Copy Markdown
Contributor
  • Shards: I dislike the idea of splitting jobs into multiple parts if the entire build step is repeated. In GitLab CI, we used to have a build step and could then copy artifacts to the next stages that could trivially be parallelized. GitHub doesn't work this way, and repeating the build 3 times defeats the purpose. We don't just want shorter parallel builds, we also want less resource usage.

not sure I understood this 100%, but from my understanding github actions allows to re-use artifacts from other build jobs using actions/upload-artifact, actions/download-artifact within a workflow

@NickSdot

Copy link
Copy Markdown
ContributorAuthor

Thanks for the review @iluuu1994

Wasn't aware of this PR. I am not sure I am following, "too many cache entries were pushed" -- do you mean that also from a resource usage point of view? Or what? My first impulse actually is that what we do here further optimises what you intended? Though, I might misunderstand. An alternative could be to keep the (easy stale going) weekly cache dropped, but not additionally have PR caches, so that PR always use fresh master caches. Would this address what you have in mind? I could benchmark how much slower it is.

  • Shards: I dislike the idea of splitting jobs into multiple parts if the entire build step is repeated. In GitLab CI, we used to have a build step and could then copy artifacts to the next stages that could trivially be parallelized. GitHub doesn't work this way, and repeating the build 3 times defeats the purpose. We don't just want shorter parallel builds, we also want less resource usage.

That's a trade off I actively decided for because the repo is on the free plan. So as long as we we don't pay for it I'd argue it's good. The build step is not the most expensive. Everyones time is also a resource. :) If we ever switch to bigger runners it would anyway be a different story and we would have other possibilities opening up, and then rethink things? It's not that we take an irreversible decision if we decide to currently benefit from the speedup.

@NickSdot

Copy link
Copy Markdown
ContributorAuthor
  • Shards: I dislike the idea of splitting jobs into multiple parts if the entire build step is repeated. In GitLab CI, we used to have a build step and could then copy artifacts to the next stages that could trivially be parallelized. GitHub doesn't work this way, and repeating the build 3 times defeats the purpose. We don't just want shorter parallel builds, we also want less resource usage.

not sure I understood this 100%, but from my understanding github actions allows to re-use artifacts from other build jobs using actions/upload-artifact, actions/download-artifact within a workflow

ASAN builds are so big that I assume sending things around, incl. compressing, uploading, downloading, and extracting would bring back a relevant performance hit. Do you have any experience with that? Any input welcome!

@iluuu1994

Copy link
Copy Markdown
Member

I am not sure I am following, "too many cache entries were pushed"

Effectively, because cache entries from old commits are not removed, cache entries accumulate very quickly and evict relevant cache entries from other branches (there's a 10 GB limit), making the cache completely ineffective.

That's a trade off I actively decided for because the repo is on the free plan. So as long as we we don't pay for it I'd argue it's good. The build step is not the most expensive.

I do think we should be mindful of how much energy we're wasting. This is adding 5 build steps, so not trivial. If we can re-use the artifacts, then maybe something to consider.

but from my understanding github actions allows to re-use artifacts from other build jobs using actions/upload-artifact, actions/download-artifact within a workflow

Possibly. Do you know if this will further put pressure on the cache, or can these artifacts be made temporary for just the current workflow run? We'd also need to make sure the jobs run a compatible image (e.g. when the runner image is partially rolled out).

@shivammathurshivammathur left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@NickSdot

  • The refactor to add build_with_test_database_setup and setup_test_databases scripts needs to be a separate PR targetting PHP-8.2, otherwise the nightly workflow would fail when this is merged.

  • This increases cache saves from weekly to per commit and will result in a lot of cache evictions, as @iluuu1994 mentioned. Also, stable-branch builds use the master commit SHA in their cache keys instead of the checked-out branch SHA.

  • sccache currently does nothing for Windows ASAN builds: all compiler calls are reported as non-cacheable. I would say sccache can be skipped for Windows ASAN unless fixed.

  • Can you check whether --enable-parallel-build with jom supports incremental builds? It would be useful for local builds as well. Currently, it appears that a second build without any changes would rebuild all objects.

  • The --enable-parallel-build argument says that it supports a parallel NMAKE-compatible tool, but since this PR only supports jom, it would be better to be clear.

  • It might be better to have jom as part of binaries in php/php-sdk-binary-tools instead of fetching it separately on each CI run.

@NickSdot

Copy link
Copy Markdown
ContributorAuthor

Thanks for the review @shivammathur!

I'll wait a bit longer to see whether more feedback comes in, and then address and look into everything in one go. 🫡

@NickSdot

Copy link
Copy Markdown
ContributorAuthor

Does anyone else have any other feedback than what was already given? I plan to get on it tomorrow.

@NickSdot
NickSdot marked this pull request as draft August 10, 2026 17:55
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants

@NickSdot@iluuu1994@staabm@arnaud-lb@shivammathur@Girgias