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][txn] Avoid txn id primitive auto boxing #18270

Merged

Conversation

lifepuzzlefun
Copy link
Contributor

@lifepuzzlefun lifepuzzlefun commented Oct 31, 2022

Fixes #18271

Motivation

avoid txnid auto box long type to Long in equals

Modifications

direct compare with ==

Verifying this change

  • Make sure that the change passes the CI checks.

This change is a trivial rework / code cleanup without any test coverage.

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

If the box was checked, please highlight the changes

  • Dependencies (add or upgrade a dependency)
  • The public API
  • The schema
  • The default values of configurations
  • The threading model
  • The binary protocol
  • The REST endpoints
  • The admin CLI options
  • Anything that affects deployment

Documentation

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

@lifepuzzlefun
Copy link
Contributor Author

@codelipenghui @Technoboy- please take a look, thank you~

@lifepuzzlefun
Copy link
Contributor Author

lifepuzzlefun commented Oct 31, 2022

simple jmh test result

Benchmark                        Mode  Samples     Score  Score error   Units
o.e.Benchmark2.direct_equals    thrpt       25  2991.795      592.223  ops/us
o.e.Benchmark2.object_equals    thrpt       25   301.050       43.026  ops/us

@github-actions github-actions bot added the doc-not-needed Your PR changes do not impact docs label Nov 1, 2022
@codelipenghui
Copy link
Contributor

/pulsarbot run-failure-checks

fix code style
@lifepuzzlefun lifepuzzlefun changed the title [Improve][txn]Avoid txn id primitive auto boxing [improve][txn]Avoid txn id primitive auto boxing Nov 1, 2022
@lifepuzzlefun lifepuzzlefun changed the title [improve][txn]Avoid txn id primitive auto boxing [improve][txn] Avoid txn id primitive auto boxing Nov 1, 2022
@codecov-commenter
Copy link

codecov-commenter commented Nov 1, 2022

Codecov Report

Merging #18270 (cf00521) into master (0866c3a) will decrease coverage by 5.92%.
The diff coverage is 0.00%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master   #18270      +/-   ##
============================================
- Coverage     38.97%   33.04%   -5.93%     
+ Complexity     8311     4391    -3920     
============================================
  Files           683      400     -283     
  Lines         67325    43611   -23714     
  Branches       7217     4476    -2741     
============================================
- Hits          26239    14411   -11828     
+ Misses        38079    27070   -11009     
+ Partials       3007     2130     -877     
Flag Coverage Δ
unittests 33.04% <0.00%> (-5.93%) ⬇️

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

Impacted Files Coverage Δ
...oker/transaction/buffer/metadata/v2/TxnIDData.java 0.00% <0.00%> (ø)
...n/java/org/apache/pulsar/client/api/RawReader.java 0.00% <0.00%> (-100.00%) ⬇️
...ava/org/apache/pulsar/broker/admin/v1/Brokers.java 0.00% <0.00%> (-100.00%) ⬇️
...va/org/apache/pulsar/broker/admin/v1/Clusters.java 0.00% <0.00%> (-100.00%) ⬇️
.../org/apache/pulsar/broker/admin/v1/Properties.java 0.00% <0.00%> (-100.00%) ⬇️
.../apache/pulsar/broker/admin/v2/ResourceGroups.java 0.00% <0.00%> (-100.00%) ⬇️
...ar/common/naming/PartitionedManagedLedgerInfo.java 0.00% <0.00%> (-100.00%) ⬇️
...e/pulsar/broker/admin/impl/ResourceQuotasBase.java 0.00% <0.00%> (-96.43%) ⬇️
...he/pulsar/broker/service/AnalyzeBacklogResult.java 0.00% <0.00%> (-92.31%) ⬇️
.../apache/pulsar/broker/admin/v1/ResourceQuotas.java 0.00% <0.00%> (-91.43%) ⬇️
... and 417 more

Copy link
Contributor

@congbobo184 congbobo184 left a comment

Choose a reason for hiding this comment

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

good catch!

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 ready-to-test
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Avoid long type autoboxing to Long in TxnID
6 participants