Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Use Utf8.IsValid to optimize ManagedWebSocket.TryValidateUtf8#104865

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
CarnaViire merged 5 commits intodotnet:mainfromstephentoub:wsutf8
Jul 15, 2024

Conversation

stephentoub
Copy link
Member

MethodMean
Existing1,279.55 ns
Utf8IsValid65.23 ns
privateUtf8MessageState_state=new();privatebyte[]_utf8Data="""    Shall I compare thee to a summer’s day?    Thou art more lovely and more temperate:    Rough winds do shake the darling buds of May,    And summer’s lease hath all too short a date:    Sometime too hot the eye of heaven shines,    And often is his gold complexion dimm’d;    And every fair from fair sometime declines,    By chance or nature’s changing course untrimm’d;    But thy eternal summer shall not fade    Nor lose possession of that fair thou owest;    Nor shall Death brag thou wander’st in his shade,    When in eternal lines to time thou growest:    So long as men can breathe or eyes can see,    So long lives this and this gives life to thee.    """u8.ToArray();[Benchmark]publicboolExisting()=>TryValidateUtf8(_utf8Data,true,_state);[Benchmark]publicboolUtf8IsValid()=>Utf8.IsValid(_utf8Data);

omariom reacted with thumbs up emojineon-sunset, PaulusParssinen, and CarnaViire reacted with rocket emoji
@dotnet-policy-serviceDotnet Policy Service
Copy link
Contributor

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

@EgorBo
Copy link
Member

Does such a big difference imply that the currently used algorithm is suboptimal? (for non-complete data)

@stephentoub
Copy link
MemberAuthor

stephentoub commentedJul 14, 2024•
edited
Loading

Does such a big difference imply that the currently used algorithm is suboptimal? (for non-complete data)

There's no vectorization or ASCII fast path, but rather multiple operations per byte.

I don't know how common it is though to split text messages into multiple fragments. If it's important, we could easily accelerate through ASCII portions. For anything more complicated, I'd prefer we expose an OperationStatus-based (or similar) validation API on Utf8 this could just use.

I added the fast part I added as I expect it's the most important case, and Utf8 makes it trivial to handle better.

Copy link
Member

@CarnaViireCarnaViire left a comment•
edited
Loading

Choose a reason for hiding this comment

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

LGTM, thanks!

But it seems like we (historically) don't have any tests for this method (because they were left inhttps://github.com/aspnet/WebSockets/blob/aa63e27fce2e9202698053620679a9a1059b501e/test/Microsoft.AspNetCore.WebSockets.Protocol.Test/Utf8ValidationTests.cs)

Can you please bring them in? And see if they also cover the changed (ascii skip) part? Thanks!

@stephentoub
Copy link
MemberAuthor

Can you please bring them in? And see if they also cover the changed (ascii skip) part? Thanks!

Done. Thanks.

Copy link
Member

@CarnaViireCarnaViire left a comment

Choose a reason for hiding this comment

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

:shipit:

@CarnaViireCarnaViire merged commit3212e3f intodotnet:mainJul 15, 2024
83 checks passed
@stephentoubstephentoub deleted the wsutf8 branchJuly 22, 2024 16:12
@github-actionsgithub-actionsbot locked and limited conversation to collaboratorsAug 22, 2024
@karelzkarelz added this to the9.0.0 milestoneSep 3, 2024
Sign up for freeto subscribe to this conversation on GitHub. Already have an account?Sign in.
Reviewers

@CarnaViireCarnaViireCarnaViire approved these changes

Assignees

@stephentoubstephentoub

Projects
None yet
Milestone
9.0.0
Development

Successfully merging this pull request may close these issues.

4 participants
@stephentoub@EgorBo@CarnaViire@karelz
[8]ページ先頭
©2009-2025 Movatter.jp