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

Jump to bottom

image/gif: FuzzDecode test runtime is excessive on various builders #55839

Closed
4a6f656c opened this issue Sep 23, 2022 · 4 comments
Closed

image/gif: FuzzDecode test runtime is excessive on various builders #55839

4a6f656c opened this issue Sep 23, 2022 · 4 comments
Labels
FrozenDueToAge help wanted NeedsFix The path to resolution is known, but the work has not been done. Testing An issue that has been verified to require only test changes, not just a test failure.
Milestone
Backlog

Comments

@4a6f656c
Copy link
Contributor

4a6f656c commented Sep 23, 2022 •
edited

The image/gif.FuzzDecode tests take an excessive amount of time to run on various builders, even in short mode.

For example, on one of the linux/riscv64 builders:

$ ./gif.test -test.v -test.short
...
=== RUN   FuzzDecode
=== RUN   FuzzDecode/seed#0
=== RUN   FuzzDecode/seed#1
=== RUN   FuzzDecode/seed#2
=== RUN   FuzzDecode/seed#3
=== RUN   FuzzDecode/seed#4
--- PASS: FuzzDecode (286.02s)
    --- PASS: FuzzDecode/seed#0 (133.46s)
    --- PASS: FuzzDecode/seed#1 (2.94s)
    --- PASS: FuzzDecode/seed#2 (63.03s)
    --- PASS: FuzzDecode/seed#3 (66.32s)
    --- PASS: FuzzDecode/seed#4 (17.93s)

While I'm all for fuzzing, this is adding almost 5 minutes to each test run on this platform.

$ go tool pprof /tmp/p
File: gif.test
Type: cpu
Time: Sep 23, 2022 at 3:47pm (UTC)
Duration: 298.35s, Total samples = 284.61s (95.39%)
Entering interactive mode (type "help" for commands, "o" for options)
(pprof) top 10
Showing nodes accounting for 277.86s, 97.63% of 284.61s total
Dropped 234 nodes (cum <= 1.42s)
Showing top 10 nodes out of 23
      flat  flat%   sum%        cum   cum%
   100.01s 35.14% 35.14%    100.69s 35.38%  image/draw.sqDiff (inline)
    96.41s 33.87% 69.01%    272.35s 95.69%  image/draw.drawPaletted
    77.11s 27.09% 96.11%     77.11s 27.09%  runtime.memmove
     1.44s  0.51% 96.61%      2.35s  0.83%  compress/lzw.(*Writer).Write
     1.41s   0.5% 97.11%      4.25s  1.49%  compress/lzw.(*Reader).decode
     0.97s  0.34% 97.45%      2.25s  0.79%  image/draw.drawPaletted.func1
     0.35s  0.12% 97.57%      1.51s  0.53%  compress/lzw.(*Reader).readLSB
     0.14s 0.049% 97.62%      7.45s  2.62%  image/gif.(*decoder).readImageDescriptor
     0.01s 0.0035% 97.62%      2.77s  0.97%  image/gif.(*encoder).writeImageBlock
     0.01s 0.0035% 97.63%      2.95s  1.04%  image/gif.EncodeAll

This also shows on other platforms such as (total image/gif runtime, however FuzzDecode makes up the majority of the runtime):

freebsd/arm - 408.816s
linux/mips - 209.886s
linux/mips64 - 174.648s
openbsd/arm - 245.098s
openbsd/mips64 - 263.389s

I've not dug into the fuzzer, however at a quick glance it appears to be CPU bound on a single OS thread. I also know that a number of the above mentioned platforms are I/O limited (running on either SDMMC, USB or iSCSI) - not sure if this plays a part.

It seems that we should do at least one of the following:

  1. Improve the performance of the fuzzing runs.
  2. Reduce the amount of work that is being done in short mode.
  3. Skip the fuzzing on a subset of platforms.
@4a6f656c
Copy link
Contributor Author

/cc @rolandshoemaker

@cherrymui cherrymui added NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Testing An issue that has been verified to require only test changes, not just a test failure. labels Sep 23, 2022
@cherrymui cherrymui added this to the Backlog milestone Sep 23, 2022
@bcmills
Copy link
Contributor

bcmills commented Sep 26, 2022

While I'm all for fuzzing, this is adding almost 5 minutes to each test run on this platform.

Note that this isn't even fuzzing — it's regression-testing the existing inputs in src/image/testdata, which should be deterministic anyway.

Probably FuzzDecode should just call f.Skip if testing.Short() is true?

@bcmills bcmills added NeedsFix The path to resolution is known, but the work has not been done. help wanted labels Sep 26, 2022
@gopherbot gopherbot removed the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Sep 26, 2022
@rolandshoemaker
Copy link
Member

Probably FuzzDecode should just call f.Skip if testing.Short() is true?

Yup seems reasonable, I vaguely remember having done this elsewhere before.

@gopherbot
Copy link

Change https://go.dev/cl/435175 mentions this issue: image/gif,image/jpeg,image/png: skip FuzzDecode in testing short mode

@4a6f656c 4a6f656c mentioned this issue Oct 2, 2022
Open
@golang golang locked and limited conversation to collaborators Sep 27, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge help wanted NeedsFix The path to resolution is known, but the work has not been done. Testing An issue that has been verified to require only test changes, not just a test failure.
Projects
None yet
Milestone
Backlog
Development

No branches or pull requests
5 participants
@rolandshoemaker @bcmills @4a6f656c @gopherbot @cherrymui