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

perf(s3stream): optimize StreamRecordBatchList to reduce object creation #903

Merged
merged 3 commits into from
Jan 30, 2024

Conversation

lifepuzzlefun
Copy link
Contributor

@lifepuzzlefun lifepuzzlefun commented Jan 28, 2024

  1. remove ArrayList copy when create instance.
  2. StreamRecordBatch will implement the ComparableItem interface without create wrap object.

@CLAassistant
Copy link

CLAassistant commented Jan 28, 2024

CLA assistant check
All committers have signed the CLA.

@lifepuzzlefun lifepuzzlefun changed the title Optimize StreamRecordBatchList perf: Optimize StreamRecordBatchList Jan 28, 2024
@daniel-y
Copy link
Contributor

Hi @lifepuzzlefun thanks for your contribution. And, please check the failed actions.

@superhx superhx requested a review from SCNieh January 29, 2024 03:02
@lifepuzzlefun lifepuzzlefun changed the title perf: Optimize StreamRecordBatchList perf(s3stream): Optimize StreamRecordBatchList Jan 29, 2024
Copy link

codecov bot commented Jan 29, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (5cbd19c) 55.90% compared to head (67b1d08) 55.91%.

Additional details and impacted files
@@             Coverage Diff              @@
##               main     #903      +/-   ##
============================================
+ Coverage     55.90%   55.91%   +0.01%     
- Complexity     1264     1266       +2     
============================================
  Files           133      133              
  Lines          8534     8534              
  Branches        789      789              
============================================
+ Hits           4771     4772       +1     
  Misses         3327     3327              
+ Partials        436      435       -1     
Components Coverage Δ
RocketMQ Broker ∅ <ø> (∅)
RocketMQ Common 5.80% <ø> (ø)
RocketMQ Controller 57.39% <ø> (ø)
RocketMQ Proxy 37.28% <ø> (+0.10%) ⬆️
RocketMQ Store 73.56% <ø> (-0.05%) ⬇️
RocketMQ Stream ∅ <ø> (∅)
RocketMQ Metadata 35.44% <ø> (ø)

see 2 files with indirect coverage changes

@SCNieh
Copy link
Contributor

SCNieh commented Jan 29, 2024

LGTM, @lifepuzzlefun the commit msg lint seems failed, maybe rebase & rewrite the commit msg to something like: perf(s3stream): optimize StreamRecordBatchList

  • xxx
  • xxx

@Chillax-0v0
Copy link
Contributor

@lifepuzzlefun You can use git rebase to modify commit messages:

  1. Run this command in your branch "lifepuzzlefun:optimzie_stream_record_batch_list"
git rebase --interactive e5e20802771fe93c9fc72c46597b35a960db9712^

(e5e2080 is the first commit hash in your branch, and a '^' means the commit before the specified commit)
2. In the opened file, modify "pick"s to "reword"s. Then save and quit.
3. Then you will be able to modify each commit message.
4. After all, you can run git push --force to update commits in this branch, and actions in this PR will re-run automatically.

1. remove ArrayList copy when create instance.
2. only create ComparableStreamRecordBatch when call get.
3. reduce java primitive type compare for Long.
@lifepuzzlefun lifepuzzlefun force-pushed the optimzie_stream_record_batch_list branch from 004b110 to 7087857 Compare January 29, 2024 13:54
@lifepuzzlefun lifepuzzlefun changed the title perf(s3stream): Optimize StreamRecordBatchList perf(s3stream): optimize StreamRecordBatchList to reduce object creation Jan 29, 2024
@lifepuzzlefun lifepuzzlefun force-pushed the optimzie_stream_record_batch_list branch 4 times, most recently from 80cfc03 to 03497d9 Compare January 29, 2024 14:03
1. remove ArrayList copy when create instance.
2. only create ComparableStreamRecordBatch when call get.
3. reduce java primitive type compare for Long.
@lifepuzzlefun lifepuzzlefun force-pushed the optimzie_stream_record_batch_list branch from 03497d9 to 3bbd770 Compare January 29, 2024 14:08
@lifepuzzlefun
Copy link
Contributor Author

lifepuzzlefun commented Jan 29, 2024

@SCNieh @Chillax-0v0 thanks for help. the commit message has been modified, hope the CI can pass. by the way, I think we need update the contribute guide to tell the contributors write commit message in the expected format.

@zhouxinyu
Copy link

We will merge the PR in a squash manner, so we don't need to modify the commit message one by one, just squashing all the commits to one commit is enough.

Anyway, I will remove the Lint Commit Messages action, since we don't need it anymore, sorry for your time. @lifepuzzlefun

@daniel-y daniel-y merged commit 178a58d into AutoMQ:main Jan 30, 2024
7 of 8 checks passed
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.

7 participants