Skip to content
This repository has been archived by the owner on Nov 17, 2023. It is now read-only.

[v1.4.x] Use CPUPinned context in ImageRecordIOParser2 #13990

Merged
merged 1 commit into from
Jan 25, 2019

Conversation

yuxihu
Copy link
Member

@yuxihu yuxihu commented Jan 25, 2019

This PR is to cherry-pick #13980 into 1.4.0 release, which fixes a significant performance regression introduced by #12666.

* create NDArray with CPUPinned context in ImageRecordIOParser2

* update document

* use -1 device_id as an option to create CPU(0) context

* retrigger CI

* fix cpplint error
@@ -285,9 +285,14 @@ inline bool ImageRecordIOParser2<DType>::ParseNext(DataBatch *out) {
shape_vec.push_back(param_.label_width);
TShape label_shape(shape_vec.begin(), shape_vec.end());

out->data.at(0) = NDArray(data_shape, Context::CPU(0), false,
auto ctx = Context::CPU(0);
auto dev_id = param_.device_id;
Copy link
Contributor

Choose a reason for hiding this comment

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

What if a device id is invalid or larger than the number of GPUs in the instance?

Copy link
Member Author

Choose a reason for hiding this comment

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

It will throw error in cuda: Check failed: e == cudaSuccess || e == cudaErrorCudartUnloading CUDA: invalid device ordinal

Copy link
Contributor

@apeforest apeforest left a comment

Choose a reason for hiding this comment

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

LGTM

@yuxihu
Copy link
Member Author

yuxihu commented Jan 25, 2019

@mxnet-label-bot update [pr-awaiting-merge]

@marcoabreu marcoabreu added the pr-awaiting-merge Review and CI is complete. Ready to Merge label Jan 25, 2019
@eric-haibin-lin eric-haibin-lin merged commit e999a46 into apache:v1.4.x Jan 25, 2019
@yuxihu yuxihu deleted the cpu_pinned_14x branch January 25, 2019 18:28
lanking520 pushed a commit to lanking520/incubator-mxnet that referenced this pull request Feb 18, 2019
…13990)

* create NDArray with CPUPinned context in ImageRecordIOParser2

* update document

* use -1 device_id as an option to create CPU(0) context

* retrigger CI

* fix cpplint error
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
pr-awaiting-merge Review and CI is complete. Ready to Merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants