Skip to content
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

Infinite loop in PCRE2 10.34 (affecting crystal docs) #13306

Closed
straight-shoota opened this issue Apr 12, 2023 · 4 comments · Fixed by #13311
Closed

Infinite loop in PCRE2 10.34 (affecting crystal docs) #13306

straight-shoota opened this issue Apr 12, 2023 · 4 comments · Fixed by #13311
Assignees
Labels
kind:bug A bug in the code. Does not apply to documentation, specs, etc. topic:infrastructure
Milestone

Comments

@straight-shoota
Copy link
Member

test_linux job on circleci is consistently failing due to being too long without output (latest nightly run: https://app.circleci.com/pipelines/github/crystal-lang/crystal/11797/workflows/78b79f84-502a-41d9-b2ac-9b22f6077fdd/jobs/75782/parallel-runs/0).
This is surprising because it runs successfully the entire spec suites and appears to hang at make docs. Generating the docs really should not take that long (it's usually just a couple of seconds).
The dist_docs job later (using the distribution binary) takes about 1 minute including setup.

@straight-shoota straight-shoota added kind:bug A bug in the code. Does not apply to documentation, specs, etc. topic:infrastructure labels Apr 12, 2023
@straight-shoota
Copy link
Member Author

straight-shoota commented Apr 12, 2023

Possibly related, I noticed a significant increase in build time for make docs from 21s to 31s on the latest nightly build compared to 1.7.3. But it stays well below 30 minutes.

$ docker run --rm -it -v "$(realpath ${PWD})":/app -w /app crystallang/crystal:1.7.3-build bash -c "time make docs"
Using /usr/bin/llvm-config-10 [version= 10.0.0]
./bin/crystal docs src/docs_main.cr  --project-name=Crystal --project-version=1.8.0-dev --source-refname=

real    0m21.369s
user    0m19.063s
sys     0m2.315s
$ make clean
$ docker run --rm -it -v "$(realpath ${PWD})":/app -w /app crystallang/crystal:nightly-build bash -c "time make docs"
Using /usr/bin/llvm-config-15 [version= 15.0.7]
./bin/crystal docs src/docs_main.cr  --project-name=Crystal --project-version=1.8.0-dev --source-refname=

real    0m31.217s
user    0m27.726s
sys     0m3.498s
$ docker run --rm -it -v "$(realpath ${PWD})":/app -w /app crystallang/crystal:1.7.3-build crystal --version
Crystal 1.7.3 [5df928b2a] (2023-03-15)

LLVM: 13.0.1
Default target: x86_64-unknown-linux-gnu
$ docker run --rm -it -v "$(realpath ${PWD})":/app -w /app crystallang/crystal:nightly-build crystal --version
Crystal 1.8.0-dev [d1fd6c802] (2023-04-09)

LLVM: 15.0.7
Default target: x86_64-unknown-linux-gnu
$ docker images --quiet --no-trunc crystallang/crystal:nightly-build
sha256:317f19b7832000f4cded9a3d7a1c13f9acbb63902ce20566adad55da29bfa66f

Maybe there's some unnoticed regression in the docs generator?

@straight-shoota
Copy link
Member Author

Apparently it's not just circleci, but it also happens on GitHub actions: https://github.com/crystal-lang/crystal/actions/runs/4672838750/jobs/8275434570?pr=13305
It seems to be just more consistent on circle ci. This might be related to the timout conditions (30 minutes with no output on circleci, 6 hours total runtime per step on GHA). So maybe it's just super slow and would still finish at some point after a long time.

@straight-shoota straight-shoota changed the title [CI] CircleCI job fails while building docs in test_linux Performance regression in crystal docs Apr 12, 2023
@straight-shoota straight-shoota added this to the 1.8.0 milestone Apr 12, 2023
@straight-shoota straight-shoota self-assigned this Apr 12, 2023
@straight-shoota
Copy link
Member Author

#13240 seems to be the responsible change. Not clear what exactly is going on.

@straight-shoota
Copy link
Member Author

@HertzDevil found a lead in form of a PCRE2 bug: PCRE2Project/pcre2@e0c6029
This bug only exists in PCRE2 versions 10.34 and 10.35, so it never appears with older or newer versions used in other contexts.
This should be fixed by #13311 where we exclude these PCRE2 versions from using MATCH_INVALID_UTF8 (it was only introduced in 10.34).
This is a hard bug, not a performance regression.

There's also a related performance regression due to the overhead of validating UTF-8. We'll fix that by allowing to tell PCRE2 that a string is valid UTF8, so it can skip that validation (cf. #13237 (comment)).

@straight-shoota straight-shoota changed the title Performance regression in crystal docs Infinite loop in PCRE2 10.34 (affecting crystal docs) Apr 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind:bug A bug in the code. Does not apply to documentation, specs, etc. topic:infrastructure
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant