Skip to content

Conversation

@stephentoub
Copy link
Member

This isn't a big deal as it's once per websocket connection, but it annoys me every time I see the unnecessary waste in a trace 😄, so I'm scratching an itch.

Previously this would allocate:

  1. A byte[] for the bytes from the guid
  2. A base64-encoded string for (1)
  3. A string concatenating (2) and the known server guid
  4. A byte[] containing the bytes from (3)
  5. A byte[] containing the SHA1-hashed bytes of (4)
  6. A base64-encoded string of (5)

Now, it only allocates (2) and (6).

Method Mean Ratio Allocated
Old 637.4 ns 1.00 472 B
New 581.1 ns 0.91 152 B

@stephentoub stephentoub added this to the 6.0.0 milestone Mar 18, 2021
@ghost
Copy link

ghost commented Mar 18, 2021

Tagging subscribers to this area: @dotnet/ncl
See info in area-owners.md if you want to be subscribed.

Issue Details

This isn't a big deal as it's once per websocket connection, but it annoys me every time I see the unnecessary waste in a trace 😄, so I'm scratching an itch.

Previously this would allocate:

  1. A byte[] for the bytes from the guid
  2. A base64-encoded string for (1)
  3. A string concatenating (2) and the known server guid
  4. A byte[] containing the bytes from (3)
  5. A byte[] containing the SHA1-hashed bytes of (4)
  6. A base64-encoded string of (5)

Now, it only allocates (2) and (6).

Method Mean Ratio Allocated
Old 637.4 ns 1.00 472 B
New 581.1 ns 0.91 152 B
Author: stephentoub
Assignees: -
Labels:

area-System.Net

Milestone: 6.0.0

This isn't a big deal as it's once per websocket connection, but it annoys me every time I see the unnecessary waste in a trace.  Previously this would allocate:
1. A byte[] for the bytes from the guid
2. A base64-encoded string for (1)
3. A string concatenating (2) and the known server guid
4. A byte[] containing the bytes from (3)
5. A byte[] containing the SHA1-hashed bytes of (4)
6. A base64-encoded string of (5)

Now, it only allocates (2) and (6).
@davidfowl
Copy link
Member

@BrennanConroy did the same on the server side 👍🏾

Copy link
Member

@wfurt wfurt left a comment

Choose a reason for hiding this comment

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

LGTM

@stephentoub stephentoub merged commit 1186816 into dotnet:main Mar 22, 2021
@stephentoub stephentoub deleted the wskeyalloc branch March 22, 2021 10:33
@ghost ghost locked as resolved and limited conversation to collaborators Apr 21, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants