Skip to content

MAINT: Disable use_hugepages in case of ValueError#16708

Merged
seberg merged 1 commit intonumpy:masterfrom
anirudh2290:fix_hugepages_issue
Jun 30, 2020
Merged

MAINT: Disable use_hugepages in case of ValueError#16708
seberg merged 1 commit intonumpy:masterfrom
anirudh2290:fix_hugepages_issue

Conversation

@anirudh2290
Copy link
Member

@anirudh2290 anirudh2290 commented Jun 29, 2020

In accordance with this comment: #16679 (comment) , adding try except and setting use_hugepages to 0 in case of ValueError, since the additional LooseVersion import increases the import time a bit. From my measurement, it is more than 1 ms increase in import time.

Closes #16679.

EDIT: distutils import is more than 1 ms increase in import time.

@charris charris added 00 - Bug 03 - Maintenance 09 - Backport-Candidate PRs tagged should be backported and removed 00 - Bug labels Jun 29, 2020
@charris
Copy link
Member

charris commented Jun 29, 2020

It's good to put Closes #<issue_number> in the commit message so that the corresponding issue is automatically closed when the PR is merged.

@anirudh2290
Copy link
Member Author

Thanks @charris !

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe it's worth adding a comment here to explain why the try/except is necessary (perhaps referencing the issue)? A little context might help a future reader understand why it was structured this way.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for the feedback @rossbar. i have added a comment here.

@anirudh2290 anirudh2290 force-pushed the fix_hugepages_issue branch from 98f9243 to e29f5d8 Compare June 30, 2020 00:47
@seberg
Copy link
Member

seberg commented Jun 30, 2020

Tried to time it, and its possible that importing LooseVersion adds so little overhead that it would not matter. But, I suppose the try/except is maybe better in any case, just to be on the safe side, and it is not like we should be missing a significant portion of platforms.
Thanks Anirudh.

@seberg seberg merged commit a41d513 into numpy:master Jun 30, 2020
@charris charris removed the 09 - Backport-Candidate PRs tagged should be backported label Jun 30, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

BUG: non-numeric kernel_version on the OVH platform

4 participants