Skip to content

Add configurable concurrency option #1176

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

Open
wants to merge 10 commits into
base: main
Choose a base branch
from

Conversation

jakebailey
Copy link
Member

@jakebailey jakebailey commented Jun 12, 2025

This adds an experimental --concurrency flag. Example usage:

  • --concurrency <default|auto|on|yes> - default behavior, uses 4 checkers, plus all cores for parse/bind/emit. Theoretically we could change this to some magic algorithm.
  • --concurrency <single|off|no>, --singleThreaded - one checker, single threaded parse/bind/emit.
  • --concurrency max - Uses GOMAXPROCS checkers, plus all cores for parse/bind/emit.
  • --concurrency half - Uses GOMAXPROCS/2 checkers, plus all cores for parse/bind/emit. (For me, this option is best for checking VS Code.)
  • --concurrency 10 - Uses 10 checkers, plus all cores for parse/bind/emit. Can be any number all the way up to the number of files.
  • For testing, --concurrency checker-per-file creates a checker for every file; max concurrency, for testing race conditions.

Maybe this is overkill, I can drop some of this.

@Copilot Copilot AI review requested due to automatic review settings June 12, 2025 23:38
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR introduces a configurable concurrency option, replacing the old single-threaded testing flag with a new --concurrency flag that allows detailed control over the number of checkers and threads used during compilation. Key changes include:

  • Adding a new "concurrency" field to compiler options and command line declarations.
  • Creating a new core package for parsing and handling concurrency values.
  • Updating test utilities, the compiler runner, and CI configurations to support the new flag.

Reviewed Changes

Copilot reviewed 15 out of 15 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
internal/tsoptions/parsinghelpers.go Added parsing for "concurrency" flag in compiler options.
internal/tsoptions/declscompiler.go Declared the new "concurrency" option for the compiler.
internal/testutil/harnessutil/harnessutil.go Set default concurrency from environment if not provided via flag.
internal/testrunner/compiler_runner.go Updated concurrency condition for skipping error baselines.
internal/core/concurrency.go Introduced parsing logic and tests for various concurrency values.
internal/compiler/program.go Updated program initialization to use new concurrency settings.
Herebyfile.mjs Updated environment variable usage to use "concurrency" instead of the old flag.
.github/workflows/ci.yml Updated environment variable in CI to support the new concurrency flag.

@jakebailey jakebailey force-pushed the jabaile/concurrency-options branch from 5eb5b31 to 99fc50c Compare June 12, 2025 23:42
@DanielRosenwasser
Copy link
Member

Instead of concurrency, I would recommend checkConcurrency.

@jakebailey
Copy link
Member Author

Except that this also controls the concurrency of the parse/bind/emit stages too... I wasn't happy with any name I came up with.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants