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

Regression in methodshow/print_to_string #11700

Closed
dhoegh opened this issue Jun 13, 2015 · 10 comments
Closed

Regression in methodshow/print_to_string #11700

dhoegh opened this issue Jun 13, 2015 · 10 comments
Labels
performance Must go faster regression Regression in behavior compared to a previous version

Comments

@dhoegh
Copy link
Contributor

dhoegh commented Jun 13, 2015

I have observed a regression of a factor of 2-3 between master and 0.3.8 in the following test case:

function test1(n)
    for i=1:n
        string(methods(max))
    end
end
test1(1)
@time test1(10000)

0.3.8 runs in 2.467998002 s while master takes 7.016 s.

@IainNZ
Copy link
Member

IainNZ commented Jun 13, 2015

Isn't this function only used interactively? And isn't the difference less than a human can perceive?

@dhoegh
Copy link
Contributor Author

dhoegh commented Jun 13, 2015

Yes, but I do not think the root cause is inside the methodshow but in the string conversion. I have not been able to track the exact difference down.

@dhoegh
Copy link
Contributor Author

dhoegh commented Jun 13, 2015

The regression occurred between 8fc5b4e and c6d0759 where no change have been made in methodshow.jl, but #10380 where merged in this interval.

I hope someone is able to track down the exact regression.

@KristofferC
Copy link
Member

Maybe you can test the commit before and see if it is fast again then? Or alternative just do a full fledged bisection.

@dhoegh
Copy link
Contributor Author

dhoegh commented Jun 13, 2015

I can confirm that the time goes from 2.3s to 3.7s at 9da1662 while the time is 6.3 at b37323c.

@dhoegh
Copy link
Contributor Author

dhoegh commented Jun 13, 2015

I have tried to track this further. The following goes from 14.7s to 20.6s post #10380

function test(n)
           a=rand(300)
           for i=1:n
               Base.print_to_string(a...)
       end
end
test(100)
@time test(100000)

@dhoegh dhoegh changed the title Regression in methodshow Regression in methodshow/print_to_string Jun 14, 2015
@JeffBezanson JeffBezanson added performance Must go faster regression Regression in behavior compared to a previous version labels Jun 14, 2015
@JeffBezanson
Copy link
Member

My best guess is that this is caused by the call to jl_f_tuple used to allocate varargs tuples (

builder.CreateCall3(prepare_call(jltuple_func), V_null,
). It now has extra overhead for constructing the type of the tuple, and then the tuple itself. This can be fixed by constructing the type at compile time, since we usually know it.

Another possibility is that the optimization to elide varargs tuples has regressed in some cases.

@JeffBezanson
Copy link
Member

Now an additional ~5x slower, with all the time in is_exported_from_stdlib.

@mbauman
Copy link
Member

mbauman commented Sep 7, 2015

I'm on a phone so I can't measure it, but Jeff already fixed at least some of it: c2d35bb (thanks Jeff!)

@KristofferC
Copy link
Member

0.3.12 runs #11700 (comment) at 10 seconds, master at 11.5 seconds. Seems close enough to close.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
performance Must go faster regression Regression in behavior compared to a previous version
Projects
None yet
Development

No branches or pull requests

5 participants