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

Set cutoff threshold for functions that rely on BLAS Level 1 routines (Close #6951) #6959

Merged
merged 2 commits into from
May 25, 2014
Merged

Conversation

lindahua
Copy link
Contributor

This patch adds a threshold for scale!, vecnorm1, and vecnorm2: they calls BLAS only when the array size is above a prefixed threshold.

This patch should fix #6112 (#6951), and ensure that these functions perform reasonably well even for small arrays.

@IainNZ
Copy link
Member

IainNZ commented May 25, 2014

Ah, I don't think this is going to fix everything in #6112

@lindahua
Copy link
Contributor Author

What was missing?

@IainNZ
Copy link
Member

IainNZ commented May 25, 2014

Well @mlubin and I aren't using these, or BLAS at all in fact. But maybe we can make a separate issue for nonBLAS perf issues.

Did these BLAS issues get worse between 0.2 and 0.3?

@lindahua lindahua changed the title Set cutoff threshold for functions that rely on BLAS Level 1 routines (Close #6112) Set cutoff threshold for functions that rely on BLAS Level 1 routines (Close #6951) May 25, 2014
@lindahua
Copy link
Contributor Author

Sorry, I actually intended to refer to #6951 (not #6112). I have corrected this above.

@staticfloat
Copy link
Member

I think these kinds of changes are fine in the short term, but in the long term, we really should have some idea of where the mystery performance drains are coming from in OpenBLAS. My investigations so far have shown that for really small problem sizes, OpenBLAS is spending a good chunk of time in memory allocation routines, so a fix might not be simple but I definitely believe that we shouldn't have to do these thresholds in Julia.

@ViralBShah
Copy link
Member

I agree that we should really try to improve OpenBLAS. For now the band aids should be OK. What would be great is if we can have a perf tuning framework to detect these cutoffs.

@ViralBShah ViralBShah added this to the 0.3 milestone May 25, 2014
@ViralBShah
Copy link
Member

How about having definitions for all these magic constants in a separate file in Base, or at least at the top somewhere?

Otherwise LGTM. @lindahua please merge when ready.

@staticfloat
Copy link
Member

That's a great idea, viral. I will do that when I do my work on tuning
parameters automatically. I don't think we need to bother this PR with
those details. :)
On May 25, 2014 12:33 AM, "Viral B. Shah" notifications@github.com wrote:

How about having definitions for all these magic constants in a separate
file in Base, or at least at the top somewhere?

Otherwise LGTM. @lindahua https://github.com/lindahua please merge when
ready.


Reply to this email directly or view it on GitHubhttps://github.com//pull/6959#issuecomment-44121170
.

@lindahua
Copy link
Contributor Author

Until better solutions happen, I think this is good enough to merge now.

lindahua added a commit that referenced this pull request May 25, 2014
Set cutoff threshold for functions that rely on BLAS Level 1 routines (Close #6951)
@lindahua lindahua merged commit 8eb9780 into JuliaLang:master May 25, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
performance Must go faster
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants