Skip to content

Conversation

@glbrntt
Copy link
Collaborator

@glbrntt glbrntt commented Jul 21, 2021

Motivation:

NIO added API for getting channel options synchronously. NIO HTTP/2
added support for this in 1.17.0. When configuring a server we configure
our new stream channels to get the stream ID to attach to the loggers.
We can now do this synchronously and save some allocations.

Modifications:

  • Bump minimum H2 version to 1.17.0
  • Use sync options to get the stream ID and sync pipeline operations to
    add the handler.

Result:

Fewer allocations.

@glbrntt glbrntt force-pushed the gb-stream-id-sync branch from 6b15298 to 0bb73e7 Compare July 21, 2021 09:34
@glbrntt glbrntt requested a review from fabianfett July 21, 2021 09:39
@glbrntt glbrntt added the 🔨 semver/patch No public API change. label Jul 21, 2021
@glbrntt glbrntt marked this pull request as ready for review July 21, 2021 09:39
Copy link
Contributor

@Davidde94 Davidde94 left a comment

Choose a reason for hiding this comment

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

Some nits, otherwise looks good.

Motivation:

NIO added API for getting channel options synchronously. NIO HTTP/2
added support for this in 1.17.0. When configuring a server we configure
our new stream channels to get the stream ID to attach to the loggers.
We can now do this synchronously and save some allocations.

Modifications:

- Bump minimum H2 version to 1.17.0
- Use sync options to get the stream ID and sync pipeline operations to
  add the handler.
- Modify alloc-limits.sh so format better aligns with new CI yaml

Result:

Fewer allocations.
@glbrntt glbrntt force-pushed the gb-stream-id-sync branch from 0bb73e7 to 3a73c69 Compare July 21, 2021 12:52
Copy link
Collaborator

@fabianfett fabianfett left a comment

Choose a reason for hiding this comment

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

LGTM

@glbrntt glbrntt merged commit 529d691 into grpc:main Jul 21, 2021
@glbrntt glbrntt deleted the gb-stream-id-sync branch July 21, 2021 14:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🔨 semver/patch No public API change.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants