Skip to content

Raw identifier syntax build regression - trunk version #239

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.

Sign up for GitHub

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

Jump to bottom
Open
dra27 wants to merge 1 commit into
base: trunk
Choose a base branch
from
Open

Raw identifier syntax build regression - trunk version #239

dra27 wants to merge 1 commit into from

Conversation

@dra27
Copy link
Owner

@dra27 dra27 commented Sep 23, 2025 •
edited
Loading

From a 32-vCPU VM:

$ hyperfine -p 'git clean -dfX; git checkout {sha}; ./configure --disable-dependency-generation' -L sha 717d9ba2c8,76a2ac4b37,da1cc7acd8,5663fc6a56,25b6cc979d,dd07f7b9b4 'make -j'
Benchmark 1: make -j (sha = 717d9ba2c8)
  Time (mean ± σ):     65.664 s ±  0.563 s    [User: 307.606 s, System: 57.598 s]
  Range (min … max):   64.764 s … 66.610 s    10 runs

Benchmark 2: make -j (sha = 76a2ac4b37)
  Time (mean ± σ):     78.664 s ±  0.342 s    [User: 309.817 s, System: 57.670 s]
  Range (min … max):   78.361 s … 79.490 s    10 runs

Benchmark 3: make -j (sha = da1cc7acd8)
  Time (mean ± σ):     78.657 s ±  0.168 s    [User: 322.876 s, System: 58.644 s]
  Range (min … max):   78.453 s … 78.971 s    10 runs

Benchmark 4: make -j (sha = 5663fc6a56)
  Time (mean ± σ):     68.925 s ±  0.171 s    [User: 323.601 s, System: 59.080 s]
  Range (min … max):   68.622 s … 69.175 s    10 runs

Benchmark 5: make -j (sha = 25b6cc979d)
  Time (mean ± σ):     88.860 s ±  0.408 s    [User: 399.498 s, System: 60.469 s]
  Range (min … max):   88.307 s … 89.763 s    10 runs

Benchmark 6: make -j (sha = dd07f7b9b4)
  Time (mean ± σ):     77.913 s ±  0.420 s    [User: 400.786 s, System: 60.522 s]
  Range (min … max):   77.677 s … 79.095 s    10 runs

@dra27
Separate keywords table from the Lexer
dd07f7b 
Lexer.is_keyword was exposed as part of the support for raw identifier
syntax in OCaml 5.2. Using this function directly added a dependency
for Path and Pprintast on Lexer and transitively on Parser which did not
exist before. Unfortunately, this significantly reduces the parallelism
when compiling ocamlcommon.cmxa.

The fix here is to move the keyword_table from Lexer to a new module
Keywords and instead expose Keywords.is_keyword and additionally
Keywords.token_of_string for the Lexer itself.

There is a small wart that Parser.token is required, but can't be
directly referenced because this would create a dependency for the new
keywords.cmx on parser.cmx and undermine the fix. The solution is to
generate an additional mli-only module Keywords_token containing a copy
(with type equality) of Parser.token and use that.
@dra27 dra27 added the no-change-entry-needed Causes the check for a Changes entry to be skipped for PRs label Sep 23, 2025
@dra27 dra27 force-pushed the fix-12323-build-regression branch from c748614 to dd07f7b Compare September 23, 2025 16:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

no-change-entry-needed Causes the check for a Changes entry to be skipped for PRs

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@dra27