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

[improve][ml] Optimization method getNumberOfEntries #23576

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

hanmz
Copy link
Contributor

@hanmz hanmz commented Nov 7, 2024

Motivation

Currently, when readPosition== lastPosition‘next, getNumberOfEntries will be entered. However, this is meaningless and may affect performance.

For example, readPosition=3:0, lastPosition=3:-1, will enter the getNumberOfEntries method
image

Modifications

When readPosition is greater than lastPosition, return 0 directly.

Documentation

  • doc
  • doc-required
  • doc-not-needed
  • doc-complete

@github-actions github-actions bot added the doc-not-needed Your PR changes do not impact docs label Nov 7, 2024
@nodece
Copy link
Member

nodece commented Nov 7, 2024

/pulsarbot rerun-failure-checks

@hanmz
Copy link
Contributor Author

hanmz commented Dec 9, 2024

/pulsarbot rerun-failure-checks

@nodece
Copy link
Member

nodece commented Dec 13, 2024

/pulsarbot rerun-failure-checks

@Jason918
Copy link
Contributor

close & reopen to trigger CI

@Jason918 Jason918 closed this Dec 16, 2024
@Jason918 Jason918 reopened this Dec 16, 2024
@codecov-commenter
Copy link

codecov-commenter commented Dec 16, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 75.49%. Comparing base (bbc6224) to head (815e3b5).
Report is 794 commits behind head on master.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff              @@
##             master   #23576      +/-   ##
============================================
+ Coverage     73.57%   75.49%   +1.91%     
- Complexity    32624    37348    +4724     
============================================
  Files          1877     1946      +69     
  Lines        139502   155026   +15524     
  Branches      15299    17834    +2535     
============================================
+ Hits         102638   117030   +14392     
- Misses        28908    29383     +475     
- Partials       7956     8613     +657     
Flag Coverage Δ
inttests 27.35% <100.00%> (+2.76%) ⬆️
systests 24.38% <100.00%> (+0.05%) ⬆️
unittests 74.92% <100.00%> (+2.08%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
...che/bookkeeper/mledger/impl/ManagedCursorImpl.java 82.08% <100.00%> (+2.78%) ⬆️

... and 673 files with indirect coverage changes

@nodece
Copy link
Member

nodece commented Dec 17, 2024

@hanmz Please rebase master branch.

@github-actions github-actions bot added the PIP label Feb 12, 2025
@hanmz hanmz force-pushed the improve/getNumberOfEntries branch from 9093593 to decc815 Compare February 12, 2025 03:53
@hanmz hanmz closed this Feb 12, 2025
@hanmz
Copy link
Contributor Author

hanmz commented Feb 12, 2025

reopen to trigger CI

@hanmz hanmz reopened this Feb 12, 2025
@hanmz hanmz requested a review from nodece February 12, 2025 04:01
@hanmz
Copy link
Contributor Author

hanmz commented Feb 12, 2025

@hanmz Please rebase master branch.

Done, please help me review again

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc-not-needed Your PR changes do not impact docs PIP
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants