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

Add option for codecov and allocation tracking to be restricted by path #44359

Merged
merged 9 commits into from
Mar 3, 2022

Conversation

IanButterworth
Copy link
Member

@IanButterworth IanButterworth commented Feb 26, 2022

Replaces #43711 (triage suggested this approach as cleaner, which I agree with)
Closes #41626
Closes #36142

Enabling code coverage and allocation tracking often blows up test suite duration.
The narrowest option currently is user which is all non-core code, including stdlibs, and given that code coverage tools usually only look for coverage in the checked out directory, the vast majority of effort is usually needless.

This PR introduces a --code-coverage=@/path/to/package and the same for --track-allocation option to declare tracking of anything falling in or under the given path. i.e. can be a specific file or a directory.

Testing use of LoopVectorization, which seems particularly effected.
Quoting @chriselrod here

coverage=true also seems to cause single-threaded @turbo to be very slow.
Yet coverage=false tests take about 10-12 minutes on CI, and many of the coverage=true tests take 2+ hours

Taking a simple LoopVectorization example:

function mydotavx(a, b)
    s = 0.0
    @turbo for i  eachindex(a,b)
        s += a[i]*b[i]
    end
    s
end
% julia --code-coverage=none --project  -e "using Foo, BenchmarkTools; a = rand(256); b = rand(256); @btime Foo.mydotavx(\$a, \$b)"
  22.645 ns (0 allocations: 0 bytes)

% julia --code-coverage=@/Users/ian/Documents/GitHub/Foo.jl --project  -e "using Foo, BenchmarkTools; a = rand(256); b = rand(256); @btime Foo.mydotavx(\$a, \$b)"
  26.826 ns (0 allocations: 0 bytes)

% julia --code-coverage=user --project  -e "using Foo, BenchmarkTools; a = rand(256); b = rand(256); @btime Foo.mydotavx(\$a, \$b)"
  532.074 ns (0 allocations: 0 bytes)

% julia --code-coverage=all --project  -e "using Foo, BenchmarkTools; a = rand(256); b = rand(256); @btime Foo.mydotavx(\$a, \$b)"
  633.341 ns (0 allocations: 0 bytes)

The effects are stronger for --track-allocation for code that heavily allocates.

I'm disappointed I didn't get this done before the 1.8 freeze, as this issue prevents us running code coverage on some long-running production test suites.

cc. @efaulhaber @carstenbauer

@IanButterworth
Copy link
Member Author

IanButterworth commented Feb 26, 2022

The new coverage tests are failing due to a slight difference in a few line counts (the expected report is on top)

"SF:/Users/ian/Documents/GitHub/julia/test/testhelpers/coverage_file.jl\nDA:3,1\nDA:4,1\nDA:5,0\nDA:7,1\nDA:8,1\nDA:9,3\nDA:10,5\nDA:11,1\nDA:12,1\nDA:14,0\nDA:17,1\nDA:18,0\nDA:19,2\nDA:20,1\nDA:22,1\nDA:1234,0\nLH:12\nLF:16\nend_of_record\n", 
"SF:/Users/ian/Documents/GitHub/julia/test/testhelpers/coverage_file.jl\nDA:3,1\nDA:4,1\nDA:5,0\nDA:7,1\nDA:8,1\nDA:9,3\nDA:10,5\nDA:11,0\nDA:12,1\nDA:14,0\nDA:17,1\nDA:18,0\nDA:19,0\nDA:20,0\nDA:22,1\nDA:1234,0\nLH:9\nLF:16\nend_of_record\n"

I'm not sure what would cause a subtle difference like this.

I've added a new reference for the output with a TODO, in case the reason isn't apparent.

@IanButterworth
Copy link
Member Author

IanButterworth commented Feb 27, 2022

Perhaps if approved, this could go in to 1.8 as strictly opt-in. i.e. Pkg.test(.. coverage=true) still would default to user in 1.8 but this could still be enabled via setting an overriding julia arg?

If it works well in the wild, it could become default in 1.9?

src/codegen.cpp Outdated Show resolved Hide resolved
Co-authored-by: Jameson Nash <vtjnash@gmail.com>
src/codegen.cpp Outdated Show resolved Hide resolved
src/jloptions.c Outdated Show resolved Hide resolved
src/jloptions.c Outdated Show resolved Hide resolved
src/jloptions.c Outdated Show resolved Hide resolved
src/jloptions.c Outdated Show resolved Hide resolved
base/util.jl Outdated Show resolved Hide resolved
src/codegen.cpp Outdated Show resolved Hide resolved
src/jloptions.c Outdated
@@ -154,11 +155,18 @@ static const char opts[] =
// instrumentation options
" --code-coverage[={none*|user|all}]\n"
" Count executions of source lines (omitting setting is equivalent to `user`)\n"
" --code-coverage=@/abs/path\n"
Copy link
Member

Choose a reason for hiding this comment

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

I don't think any other file input requires an absolute path, couldn't the path be absolutified instead?

Copy link
Member Author

Choose a reason for hiding this comment

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

Good point! I was unaware how the paths were absolutified.

The nice thing now is that a simple --code-coverage=@ will track the current directory

@KristofferC KristofferC removed the backport 1.8 Change should be backported to release-1.8 label Mar 15, 2022
philbit added a commit to philbit/julia that referenced this pull request Feb 4, 2023
giordano pushed a commit that referenced this pull request Feb 5, 2023
KristofferC pushed a commit that referenced this pull request Feb 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants