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

MutableDirectBuffer methods to put string could accept CharSequence #238

Closed
tkowalcz opened this issue Jun 29, 2021 · 5 comments
Closed

Comments

@tkowalcz
Copy link
Contributor

tkowalcz commented Jun 29, 2021

What is the code change?

Currently methods on MutableDirectBuffer that treat String as ASCII strings accept java.lang.String. I encountered a case where this is a limitation and wanted to bring it up for some feedback. Methods in question:

int putStringAscii(int index, String value);
int putStringAscii(int index, String value, ByteOrder byteOrder);
int putStringWithoutLengthAscii(int index, String value);
int putStringWithoutLengthAscii(int index, String value, int valueOffset, int length);

Rationale

In environment where buffers and objects are reused to not allocate anything it seems reasonable to me to use CharSequence instead of String whenever possible.

I am dealing with Log4j2 internal API that is written to not allocate objects and I need to pass (reusable) StringBuilder to it. Then I would like to serialise it to the Agrona ring buffer (via ExpandableArrayBuffer).

Implementation

Changing the API of MutableDirectBuffer and all implementations "just work" and the project compiles and all tests pass. Nevertheless this change is in a core class and it does not appear to be a binary compatible change.

@mjpt777
Copy link
Contributor

mjpt777 commented Jun 29, 2021

This change would have a performance impact. Fetching each character would be an invoke interface operation.

@tkowalcz
Copy link
Contributor Author

I remembered that CHA for interfaces was somehow fixed, but now rereading bugs in OpenJDK Jira I see that this is no the case. Pity. Anyway, thanks for your time.

@RichardWarburton
Copy link
Contributor

If it's important then you could add an overload and have one version of the method for the String, one for the CharSequence. Then anyone using Strings would get the benefit of the more specific method and there's still a more general alterntive.

@tkowalcz
Copy link
Contributor Author

tkowalcz commented Jun 29, 2021

To me it seems that in environment with low allocation one would reuse buffers and not have Strings that can be passed to that API. I can manage my use case and just copy and modify ExpandableArrayBuffer.

If there is interest in any particular solution I can prototype it and provide a PR for experimentation.

@mjpt777
Copy link
Contributor

mjpt777 commented Jun 29, 2021

We would accept a PR with the overloaded methods added on the MutableDirectBuffer interface with appropriate implementations.

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

No branches or pull requests

3 participants