Skip to content

ByteBufInputStream is always allocating a StringBuilder instance#8347

Merged
normanmaurer merged 1 commit intonetty:4.1from
franz1981:no_sb_on_read_line
Oct 11, 2018
Merged

ByteBufInputStream is always allocating a StringBuilder instance#8347
normanmaurer merged 1 commit intonetty:4.1from
franz1981:no_sb_on_read_line

Conversation

@franz1981
Copy link
Copy Markdown
Contributor

Motivation:

Avoid creating any StringBuilder instance if
ByteBufInputStream::readLine isn't used

Modifications:

The StringBuilder instance is lazy allocated on demand and
are added new test case branches to address the increased
complexity of ByteBufInputStream::readLine

Result:

Reduced GC activity if ByteBufInputStream::readLine isn't used

@netty-bot
Copy link
Copy Markdown

Can one of the admins verify this patch?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use StringUtil.EMPTY_STRING

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as below

@normanmaurer
Copy link
Copy Markdown
Member

@netty-bot test this please

Motivation:

Avoid creating any StringBuilder instance if
ByteBufInputStream::readLine isn't used

Modifications:

The StringBuilder instance is lazy allocated on demand and
are added new test case branches to address the increased
complexity of ByteBufInputStream::readLine

Result:

Reduced GC activity if ByteBufInputStream::readLine isn't used
@franz1981
Copy link
Copy Markdown
Contributor Author

@normanmaurer Done, thanks!

@normanmaurer normanmaurer self-assigned this Oct 11, 2018
@normanmaurer normanmaurer added this to the 4.1.31.Final milestone Oct 11, 2018
@normanmaurer normanmaurer merged commit 83dc3b5 into netty:4.1 Oct 11, 2018
@normanmaurer
Copy link
Copy Markdown
Member

@franz1981 thanks!

@franz1981 franz1981 deleted the no_sb_on_read_line branch October 11, 2018 07:05
njhill added a commit to njhill/netty that referenced this pull request Oct 12, 2018
Motivation:

While looking at the nice optimization done in
netty#8347 I couldn't help noticing the
logic could be simplified further. Apologies if this is just my OCD and
inappropriate!

Modifications:

Reduce amount of code used for ByteBufInputStream.readLine()

Result:

Slightly smaller and simpler code
normanmaurer pushed a commit that referenced this pull request Oct 13, 2018
Motivation:

While looking at the nice optimization done in
#8347 I couldn't help noticing the
logic could be simplified further. Apologies if this is just my OCD and
inappropriate!

Modifications:

Reduce amount of code used for ByteBufInputStream.readLine()

Result:

Slightly smaller and simpler code
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.

3 participants