-
Notifications
You must be signed in to change notification settings - Fork 1.9k
IGNITE-13856 make linear performance for DirectByteBufferStreamImplV2.writeString #8577
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
Changes from 13 commits
33bb73a
99122ef
b9bf2e5
d633d66
5904642
116c61d
b8dcc0f
2c7adf0
17fd38e
55e0663
2dcf0ee
542385c
90e7e6c
ed6d89f
f5225e4
bf57acb
da5c77b
ad5a952
a09987a
65859a0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -22,6 +22,7 @@ | |
| import java.util.ArrayList; | ||
| import java.util.BitSet; | ||
| import java.util.Collection; | ||
| import java.util.HashMap; | ||
| import java.util.Iterator; | ||
| import java.util.List; | ||
| import java.util.Map; | ||
|
|
@@ -301,6 +302,9 @@ public class DirectByteBufferStreamImplV2 implements DirectByteBufferStream { | |
| /** */ | ||
| protected boolean lastFinished; | ||
|
|
||
| /** map for cashing byte-array representations of strings */ | ||
| private Map<String, byte[]> stringsMap; | ||
|
|
||
| /** | ||
| * @param msgFactory Message factory. | ||
| */ | ||
|
|
@@ -584,7 +588,29 @@ public DirectByteBufferStreamImplV2(MessageFactory msgFactory) { | |
|
|
||
| /** {@inheritDoc} */ | ||
| @Override public void writeString(String val) { | ||
| writeByteArray(val != null ? val.getBytes() : null); | ||
| if (val != null) { | ||
| if (buf.capacity() < val.length()) { | ||
|
||
| if (stringsMap == null) | ||
| stringsMap = new HashMap<>(); | ||
|
|
||
| byte[] bytes = stringsMap.computeIfAbsent(val, s -> s.getBytes()); | ||
|
|
||
| try { | ||
| writeByteArray(bytes); | ||
| } | ||
| catch (Exception e) { | ||
|
||
| stringsMap.remove(val); | ||
| throw e; | ||
| } | ||
|
|
||
| if (lastFinished) | ||
| stringsMap.remove(val); | ||
| } | ||
| else | ||
| writeByteArray(val.getBytes()); | ||
| } | ||
| else | ||
| writeByteArray(null); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can we perhaps declare byteArr a local variable, and only assign it to curStrBackingArr if not lastFinished? This way we will not touch this variable at all when string fits one frame.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Actually, we need to reset it to null if lastFinished. Need to figure out the logic, maybe it's too cumbersome and confusing to implement.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Disregard this, we are going to read curStrBackingArr value either way. I think we may skip this change entirely, just rename var / mark volatile. |
||
| } | ||
|
|
||
| /** {@inheritDoc} */ | ||
|
|
||
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.
Why are you using a map here? A simple byte array is enough, isn't it?