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

Fix missing type parameter in _array_for #18009

Merged
merged 1 commit into from
Aug 15, 2016
Merged

Conversation

timholy
Copy link
Sponsor Member

@timholy timholy commented Aug 13, 2016

This fixes a performance regression in the lucompletepiv benchmark triggered by generalizing _array_for to work with indices. Ref #17960 (comment).

This call, first introduced in b363cc7, likely always triggered dynamic method dispatch. Unfortunately, the generalization to indices seemed to make that worse. This new version should be type stable and improves the performance of that benchmark by almost exactly twofold.

This fixes a performance regression in the lucompletepiv benchmark triggered by generalizing _array_for to work with indices. This call, first introduced in b363cc7, likely always triggered dynamic method dispatch. The generalization to indices seemed to make that worse.
@timholy
Copy link
Sponsor Member Author

timholy commented Aug 13, 2016

Worth checking with @JeffBezanson on whether the original design was part of a strategy to reduce specializations (for compiler performance).

@tkelman
Copy link
Contributor

tkelman commented Aug 13, 2016

yay. let's verify @nanosoldier runbenchmarks(ALL, vs = ":master")

@nanosoldier
Copy link
Collaborator

Your benchmark job has completed - possible performance regressions were detected. A full report can be found here. cc @jrevels

@timholy
Copy link
Sponsor Member Author

timholy commented Aug 13, 2016

None of the apparent regressions are reproducible for me, but the performance improvements on lucompletepiv are.

@timholy timholy merged commit db1b235 into master Aug 15, 2016
@timholy timholy deleted the teh/perf_lucompletepiv branch August 15, 2016 09:33
tkelman pushed a commit that referenced this pull request Aug 20, 2016
This fixes a performance regression in the lucompletepiv benchmark triggered by generalizing _array_for to work with indices. This call, first introduced in b363cc7, likely always triggered dynamic method dispatch. The generalization to indices seemed to make that worse.

(cherry picked from commit cb623f8)
ref #18009
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.

None yet

3 participants