Skip to content

Conversation

pjfanning
Copy link
Member

Scala 2 and 3 have different ways to inline so I split the classes into Scala 2 and 3 variants.

@pjfanning pjfanning added this to the 2.0.0 milestone Oct 6, 2025
@pjfanning pjfanning marked this pull request as draft October 6, 2025 22:09
@pjfanning pjfanning marked this pull request as ready for review October 6, 2025 22:58
Copy link
Contributor

@jrudolph jrudolph left a comment

Choose a reason for hiding this comment

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

Not worth it without a proper benchmark showing significant benefits. For these simple and small methods, static inlining is most likely a net negative with increases code size, duplication and complexity vs. a most obvious inlining opportunity in the jit inliner.

* Used in places where we know that the index is valid because we checked the length beforehand.
*/
@inline
private[http] def safeByteChar(input: ByteString, ix: Int): Char =
Copy link
Member Author

Choose a reason for hiding this comment

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

@jrudolph thanks for the review. I can look at doing a jmh benchmark but it will take me a week or 2 to get back to it.

One question though: do you think the safeByteChar function is a useful change instead of having a few places in our code where we do the masking in place while usually, we call the byteChar function below? There are places where we know it safe so that we don't need to waste a call on checking the index is in bounds.

@pjfanning pjfanning marked this pull request as draft October 10, 2025 13:26
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