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

benchmark fixture - static object pointer fix #10145

Merged
merged 1 commit into from
Jan 27, 2022

Conversation

karthikeyann
Copy link
Contributor

@karthikeyann karthikeyann commented Jan 27, 2022

benchmark fixture - fix sharing static object pointer, replace with static shared pointer.

benchmark fixture - fix static object pointer, replace with static shared pointer.
@karthikeyann karthikeyann added bug Something isn't working 3 - Ready for Review Ready for review by team tests Unit testing for project libcudf Affects libcudf (C++/CUDA) code. non-breaking Non-breaking change labels Jan 27, 2022
@karthikeyann karthikeyann requested a review from vuule January 27, 2022 05:28
@karthikeyann karthikeyann self-assigned this Jan 27, 2022
@karthikeyann karthikeyann requested a review from a team as a code owner January 27, 2022 05:28
@karthikeyann karthikeyann requested a review from trxcllnt January 27, 2022 05:28
static rmm::mr::pool_memory_resource pool_mr{&cuda_mr};
return std::shared_ptr<rmm::mr::device_memory_resource>(&pool_mr);
static auto pool_mr =
std::make_shared<rmm::mr::pool_memory_resource<rmm::mr::cuda_memory_resource>>(&cuda_mr);
Copy link
Contributor

Choose a reason for hiding this comment

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

what is the difference, was the returned type incorrect?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In old code, address of the static object pool_mr goes into shared_pointer. (pointer of a local variable should not be held in shared_pointer). when that shared pointer is destroyed, it tries to delete the pointer but it's a static object, we are getting the error.

In this new code, I created an pool_memory_resource object (in heap) and made the shared_pointer as static. So, this shared pointer object will live until program termination. When that shared pointer object is deleted at program end, the pool_memory_resource object will be deleted.

@codecov
Copy link

codecov bot commented Jan 27, 2022

Codecov Report

Merging #10145 (f101c60) into branch-22.04 (e24fa8f) will increase coverage by 0.00%.
The diff coverage is n/a.

❗ Current head f101c60 differs from pull request most recent head 549cd96. Consider uploading reports for the commit 549cd96 to get more accurate results
Impacted file tree graph

@@              Coverage Diff              @@
##           branch-22.04   #10145   +/-   ##
=============================================
  Coverage         10.37%   10.38%           
=============================================
  Files               119      119           
  Lines             20149    20207   +58     
=============================================
+ Hits               2091     2099    +8     
- Misses            18058    18108   +50     
Impacted Files Coverage Δ
python/cudf/cudf/errors.py 0.00% <0.00%> (ø)
python/cudf/cudf/io/csv.py 0.00% <0.00%> (ø)
python/cudf/cudf/io/orc.py 0.00% <0.00%> (ø)
python/cudf/cudf/__init__.py 0.00% <0.00%> (ø)
python/cudf/cudf/api/types.py 0.00% <0.00%> (ø)
python/cudf/cudf/core/frame.py 0.00% <0.00%> (ø)
python/cudf/cudf/core/index.py 0.00% <0.00%> (ø)
python/cudf/cudf/io/parquet.py 0.00% <0.00%> (ø)
python/cudf/cudf/core/dtypes.py 0.00% <0.00%> (ø)
python/cudf/cudf/core/scalar.py 0.00% <0.00%> (ø)
... and 25 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b290ec7...549cd96. Read the comment docs.

@vuule
Copy link
Contributor

vuule commented Jan 27, 2022

@gpucibot merge

@rapids-bot rapids-bot bot merged commit 5dd1c39 into branch-22.04 Jan 27, 2022
@karthikeyann karthikeyann deleted the karthikeyann-patch-4 branch May 14, 2022 20:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3 - Ready for Review Ready for review by team bug Something isn't working libcudf Affects libcudf (C++/CUDA) code. non-breaking Non-breaking change tests Unit testing for project
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants