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

[broker] Fix inefficient forEach loop #13742

Merged
merged 1 commit into from
Jan 18, 2022
Merged

[broker] Fix inefficient forEach loop #13742

merged 1 commit into from
Jan 18, 2022

Conversation

Shawyeok
Copy link
Contributor

@Shawyeok Shawyeok commented Jan 13, 2022 •
edited
Loading

Motivation

There are some methods implements with inefficient forEach loop, so fix it to get better performance. It is similar with #12953

Modifications

I rewrite it with for loop.

Verifying this change

  • Make sure that the change passes the CI checks.

This change added tests and can be verified as follows:

  • org.apache.pulsar.common.util.collections.ConcurrentSortedLongPairSetTest#testIsEmpty

Does this pull request potentially affect one of the following parts:

If yes was chosen, please highlight the changes

  • Dependencies (does it add or upgrade a dependency): (no)
  • The public API: (no)
  • The schema: (no)
  • The default values of configurations: (no)
  • The wire protocol: (no)
  • The rest endpoints: (no)
  • The admin cli options: (no)
  • Anything that affects deployment: (no)

Documentation

Check the box below or label this PR directly (if you have committer privilege).

Need to update docs?

  • no-need-doc
    It is a kind of bugfix

@github-actions github-actions bot added the doc-not-needed Your PR changes do not impact docs label Jan 13, 2022
@Shawyeok
Copy link
Contributor Author

It is similar with #12953, please take a look. @BewareMyPower @codelipenghui

@BewareMyPower BewareMyPower added area/broker release/2.8.3 release/2.9.2 type/bug The PR fixed a bug or issue reported a bug labels Jan 13, 2022
@Shawyeok
Copy link
Contributor Author

/pulsarbot rerun-failure-checks

@codelipenghui
Copy link
Contributor

/pulsarbot rerun-failure-checks

@codelipenghui codelipenghui added this to the 2.10.0 milestone Jan 14, 2022
@Shawyeok
Copy link
Contributor Author

/pulsarbot rerun-failure-checks

@Shawyeok
Copy link
Contributor Author

/pulsarbot rerun-failure-checks

mattisonchao
mattisonchao approved these changes Jan 16, 2022
View reviewed changes
Copy link
Member

@mattisonchao mattisonchao left a comment

Choose a reason for hiding this comment

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

LGTM +1

eolivelli
eolivelli approved these changes Jan 16, 2022
View reviewed changes
Copy link
Contributor

@eolivelli eolivelli left a comment

Choose a reason for hiding this comment

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

Lgtm

@codelipenghui codelipenghui force-pushed the feature/fix-inefficient-forEach-loop branch from 7878041 to 799c991 Compare January 17, 2022 08:07
@gaoran10 gaoran10 merged commit 9c94cd7 into apache:master Jan 18, 2022
codelipenghui pushed a commit that referenced this pull request Jan 18, 2022
@Shawyeok @codelipenghui
Fix inefficient forEach loop (#13742)
2804982 
### Motivation
There are some methods implemented with an inefficient forEach loop, so fix it to get better performance. It is similar to #12953

### Modifications

I rewrite it with the `for` loop.

(cherry picked from commit 9c94cd7)
@codelipenghui codelipenghui added the cherry-picked/branch-2.9 Archived: 2.9 is end of life label Jan 18, 2022
codelipenghui pushed a commit that referenced this pull request Jan 18, 2022
@Shawyeok @codelipenghui
Fix inefficient forEach loop (#13742)
44b617b 
### Motivation
There are some methods implemented with an inefficient forEach loop, so fix it to get better performance. It is similar to #12953

### Modifications

I rewrite it with the `for` loop.

(cherry picked from commit 9c94cd7)
@codelipenghui codelipenghui added the cherry-picked/branch-2.8 Archived: 2.8 is end of life label Jan 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mattisonchao mattisonchao mattisonchao approved these changes

@Demogorgon314 Demogorgon314 Demogorgon314 approved these changes

@BewareMyPower BewareMyPower BewareMyPower approved these changes

@gaoran10 gaoran10 gaoran10 approved these changes

@hezhangjian hezhangjian hezhangjian approved these changes

@codelipenghui codelipenghui codelipenghui approved these changes

@eolivelli eolivelli eolivelli approved these changes

@Jason918 Jason918 Jason918 approved these changes

@merlimat merlimat merlimat approved these changes

@michaeljmarshall michaeljmarshall Awaiting requested review from michaeljmarshall

@congbobo184 congbobo184 Awaiting requested review from congbobo184

Assignees

@Shawyeok Shawyeok

Labels
area/broker cherry-picked/branch-2.8 Archived: 2.8 is end of life cherry-picked/branch-2.9 Archived: 2.9 is end of life doc-not-needed Your PR changes do not impact docs release/2.8.3 release/2.9.2 type/bug The PR fixed a bug or issue reported a bug
Projects
None yet
Milestone
2.10.0
Development

Successfully merging this pull request may close these issues.
10 participants
@Shawyeok @codelipenghui @merlimat @Jason918 @eolivelli @hezhangjian @gaoran10 @BewareMyPower @Demogorgon314 @mattisonchao