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

Improve crypto performance #683

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

yaakov-h
Copy link
Member

Benchmark:

Benchmark run against a packet about 670 bytes long.

Current code:

  • Time to encrypt an outgoing packet: 6.464us
  • Memory allocations to encrypt an outgoing packet: 6320B
  • Time to decrypt an incoming packet: 6.524us
  • Memory allocations to decrypt an incoming packet: 5208B

This branch:

  • Time to encrypt an outgoing packet: 4.054us
  • Memory allocations to encrypt an outgoing packet: 848B +size of packet
  • Time to decrypt an incoming packet: 3.688us
  • Memory allocations to decrypt an incoming packet: 1008B + size of packet

Unfortunately it seems that about 2/3rds of the CPU time is spent computing the HMAC. I don't think we can do anything about that at the moment.

See also: #552

Some discussion on previous PR at #554.

@yaakov-h yaakov-h added this to the 2.3.0 milestone May 23, 2019
@yaakov-h yaakov-h added this to In progress in 2.3.0 Release via automation May 23, 2019
@codecov-io
Copy link

codecov-io commented May 23, 2019

Codecov Report

Merging #683 into master will increase coverage by 2.66%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #683      +/-   ##
==========================================
+ Coverage   22.07%   24.73%   +2.66%     
==========================================
  Files          86       90       +4     
  Lines        8666     9508     +842     
  Branches      714      850     +136     
==========================================
+ Hits         1913     2352     +439     
- Misses       6627     7022     +395     
- Partials      126      134       +8
Impacted Files Coverage Δ
SteamKit2/SteamKit2/Util/CryptographicContext.cs 0% <0%> (ø)
...2/Networking/Steam3/NetFilterEncryptionWithHMAC.cs 0% <0%> (ø) ⬆️
...t2/SteamKit2/Steam/Handlers/SteamUser/Callbacks.cs 2.31% <0%> (-0.76%) ⬇️
SteamKit2/SteamKit2/Types/DepotManifest.cs 0% <0%> (ø) ⬆️
...mClient/Configuration/SteamConfigurationBuilder.cs 100% <0%> (ø) ⬆️
...SteamKit2/Networking/Steam3/WebSocketConnection.cs 0% <0%> (ø) ⬆️
...Kit2/SteamKit2/Util/SteamKitWebRequestException.cs 100% <0%> (ø)
SteamKit2/SteamKit2/Util/KeyValuePairExtensions.cs 100% <0%> (ø)
...2/SteamKit2/Steam/WebAPI/WebAPIRequestException.cs 100% <0%> (ø)
... and 20 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 49c61b6...b5b47ba. Read the comment docs.

@yaakov-h yaakov-h removed this from In progress in 2.3.0 Release Mar 12, 2020
@yaakov-h yaakov-h added this to In progress in 2.4.0 Release via automation Mar 12, 2020
@yaakov-h yaakov-h modified the milestones: 2.3.0, 2.4.0 Mar 12, 2020
@xPaw
Copy link
Member

xPaw commented Sep 16, 2020

What is this blocked on?

@yaakov-h
Copy link
Member Author

Merge conflicts, tests, and ideally I'd rather use Span<> than ArraySegment<> but that means either taking a package dependency or dropping <= netstandard2.1 I think, and I'm not sure which way we want to go on that overall.

@yaakov-h yaakov-h modified the milestones: 2.4.0, 2.5.0 Nov 15, 2021
@yaakov-h yaakov-h removed this from In progress in 2.4.0 Release Nov 17, 2021
@yaakov-h yaakov-h added this to In progress in Things that need fixing via automation Nov 17, 2021
@xPaw xPaw modified the milestones: 2.5.0, Some day (PRs welcome) Mar 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging this pull request may close these issues.

None yet

3 participants