Skip to content

Conversation

bbaa-bbaa
Copy link
Contributor

@bbaa-bbaa bbaa-bbaa commented Feb 20, 2024

Fixes: #6099

We assert that heredocClosingMarker is followed by a unicode.Space to simplify the check of heredoc closing

That's about a 10x speed increase in clusterfuzz-testcase-minimized-fuzz-format-5806400649363456.txt.

Before

$ time ./caddy fmt clusterfuzz-testcase-minimized-fuzz-format-5806400649363456.txt > /dev/null
Executed in  609.03 millis    fish           external
   usr time  721.92 millis  127.15 millis  594.77 millis
   sys time   64.97 millis   26.36 millis   38.60 millis

After

$ time ./caddy fmt clusterfuzz-testcase-minimized-fuzz-format-5806400649363456.txt > /dev/null
Executed in   51.12 millis    fish           external
   usr time   40.00 millis    0.00 micros   40.00 millis
   sys time   31.47 millis  636.00 micros   30.83 millis

@bbaa-bbaa
Copy link
Contributor Author

I couldn't reproduce the original Timeout using the minimized testcase, so it was an attempt at optimization.
It would be good if anyone can test it before merge.

@francislavoie francislavoie added the bug 🐞 Something isn't working label Feb 20, 2024
@francislavoie francislavoie added this to the v2.8.0 milestone Feb 20, 2024
@bbaa-bbaa
Copy link
Contributor Author

It would be good if anyone can test it before merge.

I have tested the new testcase clusterfuzz-testcase-fuzz-format-5806400649363456.txt, see #6099 (comment)

Before

$ time ./caddy fmt clusterfuzz-testcase-fuzz-format-5806400649363456.txt > /dev/null
________________________________________________________
Executed in    7.34 secs    fish           external
   usr time    7.30 secs  612.00 micros    7.30 secs
   sys time    0.03 secs  931.00 micros    0.03 secs

After

$ time ./caddy fmt clusterfuzz-testcase-fuzz-format-5806400649363456.txt > /dev/null
________________________________________________________
Executed in   62.86 millis    fish           external
   usr time   59.54 millis   72.00 micros   59.47 millis
   sys time   21.77 millis  972.00 micros   20.80 millis

I think this PR already has enough speed without hitting the timeout.

@francislavoie
Copy link
Member

Wow 🤯 big difference. Thanks!

@francislavoie francislavoie enabled auto-merge (squash) February 20, 2024 12:22
@francislavoie francislavoie merged commit 8bbf8ec into caddyserver:master Feb 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐞 Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

fuzz-format: Timeout in fuzz-format
2 participants