Skip to content

use reverse-iterator if possible#2326

Merged
nishantmonu51 merged 1 commit intoapache:masterfrom
navis:use-reverse-iterator
Jan 28, 2016
Merged

use reverse-iterator if possible#2326
nishantmonu51 merged 1 commit intoapache:masterfrom
navis:use-reverse-iterator

Conversation

@navis
Copy link
Copy Markdown
Contributor

@navis navis commented Jan 25, 2016

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I didn't notice this earlier, but materializing the integer array might create a lot of heap pressure. Instead, why not copy the bitmap into a temporary roaring bitmap and use its reverse iterator?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

ok

@navis navis force-pushed the use-reverse-iterator branch from 34591c0 to b719ee4 Compare January 26, 2016 01:05
@fjy
Copy link
Copy Markdown
Contributor

fjy commented Jan 27, 2016

👍 after squashing commits

@fjy fjy added this to the 0.9.0 milestone Jan 27, 2016
@xvrl
Copy link
Copy Markdown
Member

xvrl commented Jan 27, 2016

👍 once commits are squashed

@navis navis force-pushed the use-reverse-iterator branch from b719ee4 to 7324ece Compare January 28, 2016 00:05
@nishantmonu51
Copy link
Copy Markdown
Member

👍, will merge once travis passes.

nishantmonu51 added a commit that referenced this pull request Jan 28, 2016
@nishantmonu51 nishantmonu51 merged commit 99017f4 into apache:master Jan 28, 2016
@navis navis deleted the use-reverse-iterator branch January 29, 2016 01:06
@fjy fjy added the Improvement label Feb 5, 2016
seoeun25 pushed a commit to seoeun25/incubator-druid that referenced this pull request Jan 10, 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.

5 participants