Skip to content

CI: Add CTest for ApplySearch Threading - #2374

Merged
xsscx merged 2 commits into
masterfrom
ci-qa-pr-perf-test
Sep 3, 2026
Merged

CI: Add CTest for ApplySearch Threading#2374
xsscx merged 2 commits into
masterfrom
ci-qa-pr-perf-test

Conversation

@xsscx

@xsscx xsscx commented Sep 3, 2026

Copy link
Copy Markdown
Member

PR Summary

#2346

Checklist

  • Signed all Commits in PR
  • Built locally according to docs/build.md
  • Followed the guidelines in Contributing document
  • Ran relevant CTest/profile tests from docs/ctest.md
  • Updated documentation for user-visible behavior changes
  • Ran sanitizer coverage for memory-safety or parser changes
  • Added or updated regression coverage for behavior changes
  • Attached a base...HEAD contract matrix for cross-cutting changes:
    producer, consumer, build/runtime behavior, platform/toolchain boundary,
    CI trigger, dependency owner, and local evidence
  • Reviewed active and suppressed automated findings from review threads and summaries
  • For Python package changes, followed docs/python-packaging-release.md for PR and merge requirements
  • Did not change maintainer-owned workflow, CTest, CPack, sanitizer, release, or security infrastructure unless requested by an iccDEV maintainer
  • New source files include the ICC copyright and BSD 3-Clause license header
  • Code style matches nearby code: 2-space indent, K&R braces, m_ members

Legal Requirements

All official software projects hosted by the International Color Consortium (ICC)
follows the open source software best practice policies. The International Color Consortium IP policy governs ICC specification development and contributions to ICC open source software. Software contributions are also covered by the Contributor License Agreement (CLA).

Contributor License Agreements

Developers who wish to contribute code to be considered for inclusion
in ICC software must first complete a Contributor License Agreement
(CLA)
.

There is no cost or membership requirement to sign the ICC Contributor License Agreement (CLA). Please note that this is different from membership in the International Color Consortium (ICC). If your organization relies on our projects, please become a member. Membership dues are an essential source of funding and investment for these projects.

  • If you are an individual writing the code on your own time and you are SURE you are the sole owner of any intellectual property you contribute, you can sign the CLA as an individual contributor.

  • If you are writing the code as part of your job, or if there is any possibility that your employer might think they own any intellectual property you create, then you should use the Corporate Contributor Licence Agreement

License

ICC software is licensed under the BSD 3-Clause "New" or "Revised" License. Contributions to ICC software projects should abide by that license unless otherwised specified or approved by the ICC.

Copyright Notices

All new source files must begin with the ICC Copyright notice and include or reference the BSD 3-Clause "New" or "Revised" License.

INTELLECTUAL PROPERTY & PATENTS

Participation in ICC's development activities is subject to ICC's Patent Policy.

Maintainer Review Required

If you have questions, contact a listed Maintainer.

@xsscx xsscx added this to the v2.3.2.4 milestone Sep 3, 2026
@xsscx xsscx self-assigned this Sep 3, 2026
@github-actions github-actions Bot added pending CI checks still running Documentation Documentation-only or documentation-related change Tools Command-line tool or GUI tool changes Source C or C++ source code changes Scripts Shell, PowerShell, or repository automation scripts Unix Linux, macOS, Bash, or POSIX shell scope labels Sep 3, 2026
@xsscx

xsscx commented Sep 3, 2026

Copy link
Copy Markdown
Member Author

PR Author Notes

2026-09-03 18:10:29 UTC

This PR started in #2363 but Stacked PR was not helping with the Copilot Code Review Process.

@xsscx xsscx changed the title Ci qa pr perf test CI: Add CTest for ApplySearch Threading Sep 3, 2026
@xsscx xsscx assigned colourbill-ctrl and unassigned xsscx Sep 3, 2026
@xsscx xsscx linked an issue Sep 3, 2026 that may be closed by this pull request
@xsscx xsscx added Codex Codex use indicated by Maintainer gpt-daybreak-blue Maintainer indicates use of Codex gpt-daybreak-blue Model labels Sep 3, 2026
@xsscx
xsscx force-pushed the ci-qa-pr-perf-test branch from 26268ca to f01b651 Compare September 3, 2026 18:35
@github-actions github-actions Bot removed Tools Command-line tool or GUI tool changes Source C or C++ source code changes labels Sep 3, 2026
@xsscx
xsscx requested a lite review from Copilot September 3, 2026 18:49
@xsscx xsscx assigned xsscx and unassigned colourbill-ctrl Sep 3, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

The new threads-debugcalc regression currently doesn’t assert the expected diagnostic (so it can pass for unrelated failures), and the threaded fixture generation uses a hard-coded repeat count that is unnecessarily fragile.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR strengthens CI regression coverage for iccApplySearch threading by expanding the existing iccdev.applysearch-cli-args CTest to validate deterministic output across multiple -threads settings and to pin several malformed thread-count argument cases.

Changes:

  • Generate a >1024-row derived input fixture to ensure the threaded bulk path is exercised, then compare output across -threads 0/1/2/4/8.
  • Add negative tests for invalid -threads values and invalid flag usage combinations (including -debugcalc + non-1 threads).
  • Document the expanded iccdev.applysearch-cli-args coverage in docs/ctest.md.
File summaries
File Description
docs/ctest.md Documents the expanded iccdev.applysearch-cli-args threading and argument-surface coverage.
.github/scripts/iccdev-applysearch-cli-args-regression.sh Extends the regression script to exercise threaded batch apply, validate deterministic output across thread counts, and pin additional -threads rejection cases.
Review details
  • Files reviewed: 2/2 changed files
  • Comments generated: 2
  • Review effort level: Lite

Comment thread .github/scripts/iccdev-applysearch-cli-args-regression.sh Outdated
Comment thread .github/scripts/iccdev-applysearch-cli-args-regression.sh

@colourbill-ctrl colourbill-ctrl left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Reviewed at f01b651d. CI skipped every build lane on this script+docs change, so the execution evidence is local: ctest -R iccdev.applysearch-cli-args passes against master 3e2224d2 in 1.8s.

The fixture is not vacuous: 1,040 rows clears the 1,024 bulk threshold, so pixelsPerThread drops to 128 and every requested worker dispatches. Measured on that exact fixture, -threads 1/2/4/8: 0.28s / 0.21s / 0.10s / 0.07s wall, CPU rising 0.28s to 0.52s, output byte-identical throughout. Rejection cases match the tool: bare -threads, -threads 8 alone, -1, 257 (with the expected 0..256 text), nope, and -debugcalc with workers all exit non-zero.

Approving.

@xsscx
xsscx enabled auto-merge (squash) September 3, 2026 19:01
@xsscx
xsscx merged commit bdcc43a into master Sep 3, 2026
16 checks passed
@xsscx
xsscx deleted the ci-qa-pr-perf-test branch September 4, 2026 14:55
xsscx added a commit that referenced this pull request Sep 6, 2026
Signed-off-by: David Hoyt <dhoyt@hoyt.net>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Codex Codex use indicated by Maintainer Documentation Documentation-only or documentation-related change gpt-daybreak-blue Maintainer indicates use of Codex gpt-daybreak-blue Model pending CI checks still running Scripts Shell, PowerShell, or repository automation scripts Unix Linux, macOS, Bash, or POSIX shell scope

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Research: Perf: CreateSearch() accepts no thread count

3 participants