Update Text class to use native java ByteBuffer #127666
Conversation
jordan-powers
added
>non-issue
:Core/Infra/Core
|
Pinging @elastic/es-core-infra (Team:Core/Infra) |
server/src/main/java/org/elasticsearch/common/io/stream/StreamInput.java
Outdated
Show resolved
Hide resolved
libs/x-content/src/main/java/org/elasticsearch/xcontent/Text.java
Outdated
Show resolved
Hide resolved
libs/x-content/src/main/java/org/elasticsearch/xcontent/XContentString.java
Outdated
Show resolved
Hide resolved
| @@ -419,7 +419,7 @@ public void writeText(Text text) throws IOException { | |||
| writeInt(spare.length()); | |||
| write(spare.bytes(), 0, spare.length()); | |||
| } else { | |||
| BytesReference bytes = text.bytes(); | |||
| BytesReference bytes = BytesReference.fromByteBuffer(text.bytes()); | |||
There was a problem hiding this comment.
nit: we could support reading/writing ByteBuffer directly in StreamInput/Output so that conversions are not necessary here. That could allow optimizing in the ByteArrayStreamInput case to create a ByteBuffer that wraps the underlying byte array with appropriate offset/length, without needing to copy bytes to a new array.
There was a problem hiding this comment.
I'll look into that, thanks!
There was a problem hiding this comment.
Ok, I'm thinking this might be a bigger change than we want to incorporate into this PR. It might be better to leave it as a follow-up, since the current implementation will still work, it'll just be less optimized.
There was a problem hiding this comment.
I'm OK with leaving this in a follow-up, it will help with reviews too
jordan-powers
added
auto-backport
💔 Backport failed
You can use sqren/backport to manually backport by running |
💚 All backports created successfully
Questions ?Please refer to the Backport tool documentation |
This PR is a precursor to #126492. It does three things: - Move org.elasticsearch.common.text.Text from :server to org.elasticsearch.xcontent.Text in :libs:x-content. - Refactor the Text class to use a java-native ByteBuffer instead of the elasticsearch BytesReference. - Add the XContentString interface, with the Text class implementing that interface. (cherry picked from commit db0c3c7) # Conflicts: # server/src/main/java/org/elasticsearch/search/fetch/subphase/highlight/DefaultHighlighter.java # server/src/test/java/org/elasticsearch/common/xcontent/BaseXContentTestCase.java # server/src/test/java/org/elasticsearch/search/fetch/subphase/highlight/HighlightFieldTests.java # test/framework/src/main/java/org/elasticsearch/search/SearchResponseUtils.java
This PR is a precursor to elastic#126492. It does three things: - Move org.elasticsearch.common.text.Text from :server to org.elasticsearch.xcontent.Text in :libs:x-content. - Refactor the Text class to use a java-native ByteBuffer instead of the elasticsearch BytesReference. - Add the XContentString interface, with the Text class implementing that interface.
This PR is a precursor to #126492.
It does three things:
org.elasticsearch.common.text.Textfrom:servertoorg.elasticsearch.xcontent.Textin:libs:x-content.Textclass to use a java-nativeByteBufferinstead of the elasticsearchBytesReference.XContentStringinterface, with theTextclass implement that interface.