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

Speed up checkbounds #11843

Merged
merged 1 commit into from
Jun 25, 2015
Merged

Speed up checkbounds #11843

merged 1 commit into from
Jun 25, 2015

Conversation

mbauman
Copy link
Member

@mbauman mbauman commented Jun 24, 2015

This avoid the splatting allocations in checkbounds, and also ensures that the one-index version is not less specific than the n-ary version.

@mbauman mbauman mentioned this pull request Jun 24, 2015
Also ensure that the one-index version is not less specific than the n-ary version.
@mbauman mbauman force-pushed the mb/fastercheckbounds branch from ba48646 to 6a6d365 Compare June 24, 2015 20:49
@timholy
Copy link
Member

timholy commented Jun 25, 2015

LGTM

@mbauman
Copy link
Member Author

mbauman commented Jun 25, 2015

Man, I can't get anything through CI these days. Travis push failed because when I created this PR I thought I'd go easy on the travis queue and just push to my own fork. But then I forgot about that and updated the fork here on this repo, and then deleted it again. So that's not a problem. But Appveyor got stuck somewhere in the tests… @tkelman can I merge this guy?

@tkelman
Copy link
Contributor

tkelman commented Jun 25, 2015

I think you should be able to, our fix from yesterday didn't do anything to make #11818 any better. As long as we get at least some green builds after merging this I don't think it's making it any worse.

mbauman added a commit that referenced this pull request Jun 25, 2015
@mbauman mbauman merged commit 78760e2 into JuliaLang:master Jun 25, 2015
@mbauman mbauman deleted the mb/fastercheckbounds branch June 25, 2015 14:27
@mbauman
Copy link
Member Author

mbauman commented Jun 25, 2015

Thanks @timholy and @tkelman

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants