-
Notifications
You must be signed in to change notification settings - Fork 8.8k
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
add: recycle buffer #704
add: recycle buffer #704
Conversation
Codecov Report
@@ Coverage Diff @@
## develop #704 +/- ##
=============================================
+ Coverage 32.71% 32.72% +0.01%
- Complexity 895 896 +1
=============================================
Files 227 226 -1
Lines 8804 8797 -7
Branches 1061 1058 -3
=============================================
- Hits 2880 2879 -1
+ Misses 5598 5595 -3
+ Partials 326 323 -3
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i left some comments
server/src/main/java/com/alibaba/fescar/server/session/BranchSession.java
Outdated
Show resolved
Hide resolved
server/src/main/java/com/alibaba/fescar/server/session/GlobalSession.java
Outdated
Show resolved
Hide resolved
server/src/main/java/com/alibaba/fescar/server/store/FileTransactionStoreManager.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks. LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1
@@ -299,8 +303,10 @@ public void run() { | |||
|
|||
private boolean writeDataFile(byte[] bs) { | |||
int retry = 0; | |||
byte[] byWrite = new byte[bs.length + 4]; | |||
ByteBuffer byteBuffer = ByteBuffer.wrap(byWrite); | |||
ByteBuffer byteBuffer = wireteBuffer; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if size > 4096?
Ⅰ. Describe what this PR did
In the code, there are many bytebuffers created but not reused. To reduce the number of YGC, ByteBuffer is reused.
I first read netty's recycle and implemented it. But this is a big change and will make the byte[] that was previously returned by SessionStorable into one of our custom objects, so I dropped it.
Then I looked at other store's system, such as RocketMq, which use ThreadLocal for caching,.it is simple and has very little momentum.
I ran the write part of WriteStoreTest, the VM parameter was -Xmx10m, the YGC was 3,400 times before the modification, and the YGC was 900 times after the modification.
I also changed the HeapByteBuffer used by Filechannel to DirectHeapByteBuffer.
Ⅱ. Does this pull request fix one issue?
Ⅲ. Why don't you add test cases (unit test/integration test)?
Ⅳ. Describe how to verify it
run write part of WriteStoreTest, VM parameter -Xmx10m
wathching jstat -gc pid 1000
Ⅴ. Special notes for reviews