Skip to content

Comments

use right value instead of copy value in c++ interface#320

Merged
lemire merged 2 commits intoRoaringBitmap:masterfrom
stdpain:avoid_copy_desr
Sep 8, 2021
Merged

use right value instead of copy value in c++ interface#320
lemire merged 2 commits intoRoaringBitmap:masterfrom
stdpain:avoid_copy_desr

Conversation

@stdpain
Copy link
Contributor

@stdpain stdpain commented Sep 8, 2021

use move instead of copy in c++ interface which will get good performance under sparse bitmap
StarRocks/starrocks#86

@lemire lemire requested a review from Oppen September 8, 2021 15:41
@lemire
Copy link
Member

lemire commented Sep 8, 2021

I like it a lot, but I'd like an independent review before merging.

@Oppen would you have a look? It short.

@Oppen
Copy link
Collaborator

Oppen commented Sep 8, 2021

It seems insert also implements r-value reference versions, so if insertOrEmplace is only for internal use, it may be better to just add an r-value reference version and always use std::move in that function. Does that make sense?
I can probably be done that way regardless of it being internal or not, at least Godbolt won't err on a dummy case.

@stdpain
Copy link
Contributor Author

stdpain commented Sep 8, 2021

@Oppen Applied

@Oppen
Copy link
Collaborator

Oppen commented Sep 8, 2021

LGTM!

@lemire
Copy link
Member

lemire commented Sep 8, 2021

This is nice. Merging.

@lemire lemire merged commit b3d5ecc into RoaringBitmap:master Sep 8, 2021
morningman pushed a commit to apache/doris that referenced this pull request Mar 17, 2022
morningman pushed a commit to apache/doris that referenced this pull request Mar 17, 2022
morningman pushed a commit to apache/doris that referenced this pull request Mar 17, 2022
zhengte pushed a commit to zhengte/incubator-doris that referenced this pull request Mar 20, 2022
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