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

QPIDJMS-417 Reduce GC pressure while using BytesMessage #22

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

franz1981
Copy link
Contributor

Using directly ByteBuf-based streams allows to avoid
unnecessary creations of intermediate instances to
operate on the underline ByteBuf content

Using directly ByteBuf-based streams allows to avoid
unnecessary creations of intermediate instances to
operate on the underline ByteBuf content
@franz1981
Copy link
Contributor Author

@gemmellr @tabish121 I'm not very proud to have exposed directly the ByteBuf streams, but I have tried to use the right type on the facade and it makes the code really unreadable (and confuse the mocking framework too!!) :(

<I extends InputStream & DataInput> I getInputStream() throws JMSException;

@gemmellr
Copy link
Member

Can you elaborate on the benefits you measured here? I'd like to understand the extent to consider against the downside of exposing dep impl types throughout the code base.

The existing bits worked the way it did at least a small part because BytesMessage specified thats where it gets its behaviours from. Are we sure ByteBuf streams behave the same?

@franz1981
Copy link
Contributor Author

franz1981 commented Oct 10, 2018

@gemmellr

Can you elaborate on the benefits you measured here? I'd like to understand the extent to consider against the downside of exposing dep impl types throughout the code base.

Fair enough 👍 I will come up with a test to measure the reduction of GC (with 100 bytes message)

The existing bits worked the way it did at least a small part because BytesMessage specified thats where it gets its behaviours from. Are we sure ByteBuf streams behave the same?

Looking at the tests on Netty (where I have put recently a PR re some of these classes) it seems the case, but it is not part of Java util classes so they could be wrong on it. Do we have some tests to verify the impact on the client of it?

Base automatically changed from master to main March 23, 2021 04:06
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

Successfully merging this pull request may close these issues.

2 participants