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

SslHandler consolidate state to save memory #11160

Merged
merged 1 commit into from
Apr 15, 2021

Conversation

Scottmitch
Copy link
Member

Motivation:
SslHandler has many independent boolean member variables. They can be
collapsed into a single variable to save memory.

Modifications:

  • SslHandler boolean state consolidated into a single short variable.

Result:
Savings of 8 bytes per SslHandler (which is per connection) observed on
OpenJDK.

Motivation:
SslHandler has many independent boolean member variables. They can be
collapsed into a single variable to save memory.

Modifications:
- SslHandler boolean state consolidated into a single short variable.

Result:
Savings of 8 bytes per SslHandler (which is per connection) observed on
OpenJDK.
@Scottmitch
Copy link
Member Author

I need to add more state in #11156 which was the motivation for this PR.

@Scottmitch Scottmitch merged commit 16b40d8 into netty:4.1 Apr 15, 2021
@Scottmitch Scottmitch deleted the ssl_handler_state branch April 15, 2021 15:18
Scottmitch added a commit that referenced this pull request Apr 15, 2021
Motivation:
SslHandler has many independent boolean member variables. They can be
collapsed into a single variable to save memory.

Modifications:
- SslHandler boolean state consolidated into a single short variable.

Result:
Savings of 8 bytes per SslHandler (which is per connection) observed on
OpenJDK.
@Scottmitch
Copy link
Member Author

master (3049eac)

outboundBuffer.totalPendingWriteBytes() > 0);
if (fastOpen && ((outboundBuffer = channel.unsafe().outboundBuffer()) == null ||
outboundBuffer.totalPendingWriteBytes() > 0)) {
setState(STATE_NEEDS_FLUSH);

Choose a reason for hiding this comment

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

(Tracking down some Netty-related issue led me to this PR, and led me to do a close reading. I noticed this and didn't want it to get lost in case it is valuable. It is also possible I've misread something.)

On this line, the prior version has:

needsFlush |= ...

…so shouldn't there be a corresponding clearState(STATE_NEEDS_FLUSH) if the elaborate condition does not hold?

Again this is just from a flyby code read so I may have missed something deeper.

Copy link
Contributor

Choose a reason for hiding this comment

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

That state is cleared when the flush happens. The effect of |= is to assign to the LHS the bitwise-OR of LHS and RHS. This PR does not change that behaviour. It just consolidates many fields into a single bit-field.

raidyue pushed a commit to raidyue/netty that referenced this pull request Jul 8, 2022
Motivation:
SslHandler has many independent boolean member variables. They can be
collapsed into a single variable to save memory.

Modifications:
- SslHandler boolean state consolidated into a single short variable.

Result:
Savings of 8 bytes per SslHandler (which is per connection) observed on
OpenJDK.
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.

None yet

3 participants