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

Make coverage/alloc work without turning off inlining #17476

Closed
timholy opened this issue Jul 18, 2016 · 3 comments · Fixed by #18196
Closed

Make coverage/alloc work without turning off inlining #17476

timholy opened this issue Jul 18, 2016 · 3 comments · Fixed by #18196
Labels
potential benchmark Could make a good benchmark in BaseBenchmarks

Comments

@timholy
Copy link
Member

timholy commented Jul 18, 2016

Now with our improved meta information, I wonder if in principle it's possible to make coverage analysis work properly without --inline=no. Ref c68a2cd

@wookay
Copy link
Contributor

wookay commented Jul 20, 2016

I'm glad to hear that. +1. thanks

@timholy
Copy link
Member Author

timholy commented Jul 21, 2016

This may already be well-known, but I just (re)discovered an interesting problem while working on an upcoming package, IndirectArrays, in which running our tests with --inline=no causes an actual problem. Here and here I'm exploiting/testing our snazzy @boundscheck functionality. But that functionality relies on inlining, so turning it off causes Travis tests to fail. Consequently, this seems to force one to choose between testing the boundcheck removal and getting good coverage statistics.

CC @blakejohnson, in case you haven't discovered this issue yet. (To be clear, I think the fix is to improve the way we measure coverage, not to change how @boundscheck and associated infrastructure works.)

@blakejohnson
Copy link
Contributor

@timholy right, we go through some hoops to run the base boundscheck tests 3 different ways to test all code paths. As a first step, to get the test to pass with or without inlining, you can modify the test based upon the value of Base.JLOptions().can_inline, but that is obviously not ideal.

@kshyatt kshyatt added the potential benchmark Could make a good benchmark in BaseBenchmarks label Aug 9, 2016
yuyichao added a commit that referenced this issue Aug 23, 2016
yuyichao added a commit that referenced this issue Aug 23, 2016
yuyichao added a commit that referenced this issue Aug 23, 2016
yuyichao added a commit that referenced this issue Aug 23, 2016
yuyichao added a commit that referenced this issue Aug 27, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
potential benchmark Could make a good benchmark in BaseBenchmarks
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants