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 laplace_vec benchmark #5435

Closed
JeffBezanson opened this issue Jan 17, 2014 · 2 comments
Closed

regression in laplace_vec benchmark #5435

JeffBezanson opened this issue Jan 17, 2014 · 2 comments
Labels
performance Must go faster regression Regression in behavior compared to a previous version
Milestone

Comments

@JeffBezanson
Copy link
Sponsor Member

See http://speed.julialang.org/timeline/#/?exe=2,1,3&base=none&ben=laplace_vec&env=1&revs=50&equid=off

Based on the timing, this was caused by the @ngenerate changes. cc @timholy

This is really a special case of vector-related performance in general (see #1168), but it would be nice if there's something simple we can do to at least get back the old performance.

@timholy
Copy link
Sponsor Member

timholy commented Jan 17, 2014

Sorry about that. The issue was that r[i] is very slow for Ranges, so the J_d[i_d] statements were slow. I inserted a workaround. (A better fix will require inlining and/or boundscheck improvements for Ranges.) I also added an @inbounds to hopefully further boost performance, although that's much less important than fixing the indexing of Ranges.

Before:

julia> @time laplace_vec();
elapsed time: 2.383288048 seconds (1795578228 bytes allocated)

julia> @time laplace_vec();
elapsed time: 2.343158875 seconds (1795571616 bytes allocated)

Now:

julia> @time laplace_vec();
elapsed time: 1.319227933 seconds (1795571616 bytes allocated)

julia> @time laplace_vec();
elapsed time: 1.310334639 seconds (1795571616 bytes allocated)

This is a larger ratio than the loss of performance shown by codespeed; it seems possible that now we'll be even faster than before. (Either that, or it's machine-dependent.)

@JeffBezanson
Copy link
Sponsor Member Author

Sweet, thanks for fixing this so fast.

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

2 participants