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.

Sign up for GitHub

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

Jump to bottom

[REVIEW] Implement span storage optimization. #515

Merged
merged 5 commits into from
Feb 18, 2022
Merged

[REVIEW] Implement span storage optimization. #515

merged 5 commits into from
Feb 18, 2022

Conversation

trivialfis
Copy link
Member

Close #511 .

@trivialfis trivialfis added improvement Improvement / enhancement to an existing function non-breaking Non-breaking change labels Feb 17, 2022
@trivialfis trivialfis requested a review from a team as a code owner February 17, 2022 21:38
@github-actions github-actions bot added the cpp label Feb 17, 2022
@trivialfis trivialfis changed the title [ENH] Implement span storage optimization. [REVIEW] Implement span storage optimization. Feb 18, 2022
@trivialfis
Copy link
Member Author

cc @jrhemstad

@trivialfis
Address reviewer's comments.
8998252 
* Delegate the ctor.
* Use std::size_t consistently.
@trivialfis
No discard for data.
444a1d7
cjnolet
cjnolet approved these changes Feb 18, 2022
View reviewed changes
Copy link
Member

@cjnolet cjnolet left a comment

Choose a reason for hiding this comment

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

LGTM

@cjnolet
Copy link
Member

cjnolet commented Feb 18, 2022

@gpucibot merge

@rapids-bot rapids-bot bot merged commit 1409002 into rapidsai:branch-22.04 Feb 18, 2022
@trivialfis trivialfis deleted the enh-span-storage branch February 18, 2022 18:18
@trivialfis
Copy link
Member Author

trivialfis commented Feb 22, 2022 •
edited
Loading

@jrhemstad Out of curiosity, how far is this version of span implementation from the one planned for libcu++ (if it's needed for libcu++)? Do you think it takes a lot of effort to upstream this one?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jrhemstad jrhemstad jrhemstad approved these changes

@cjnolet cjnolet cjnolet approved these changes

Assignees
No one assigned
Labels
cpp improvement Improvement / enhancement to an existing function non-breaking Non-breaking change
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[FEA] Optimize span class with static extent
3 participants
@trivialfis @cjnolet @jrhemstad