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

Make connection options parsing "safe" #21004

Merged
merged 4 commits into from
Apr 29, 2020

Conversation

benaadams
Copy link
Member

@benaadams benaadams commented Apr 20, 2020

To @blowdart with ❤

Contributes to #4720

Starting PR run on '35b259753593be3e9a911b8d936348c0d4a38771'...
| Description |     RPS | CPU (%) | Memory (MB) | Avg. Latency (ms) | Startup (ms) | Build Time (ms) | Published Size (KB) | First Request (ms) | Latency (ms) | Errors | Ratio |
| ----------- | ------- | ------- | ----------- | ----------------- | ------------ | --------------- | ------------------- | ------------------ | ------------ | ------ | ----- |
|      Before | 352,614 |      99 |          92 |              6.61 |          516 |           27019 |              120355 |             161.88 |         0.58 |      0 |  1.00 |
|       After | 356,683 |      99 |          90 |              6.48 |          494 |            7506 |              120355 |             129.31 |         0.78 |      0 |  1.01 |

@ghost ghost added the area-servers label Apr 20, 2020
@benaadams
Copy link
Member Author

@aspnet-hello benchmark

@pr-benchmarks
Copy link

pr-benchmarks bot commented Apr 20, 2020

Starting 'Default' pipelined plaintext benchmark with session ID '715a1c6bbe6145c38fbd64c042e8e22c'. This could take up to 30 minutes...

@pr-benchmarks
Copy link

pr-benchmarks bot commented Apr 20, 2020

Baseline

Starting baseline run on '2d7238601c4d5894fd6f2cd68cce8e24967ef994'...
RequestsPerSecond:           342,994
Max CPU (%):                 99
WorkingSet (MB):             88
Avg. Latency (ms):           6.71
Startup (ms):                511
First Request (ms):          160.31
Latency (ms):                0.46
Total Requests:              5,178,484
Duration: (ms)               15,100
Socket Errors:               0
Bad Responses:               0
Build Time (ms):             27,023
Published Size (KB):         120,299
SDK:                         5.0.100-preview.2.20120.3
Runtime:                     5.0.0-preview.4.20218.1
ASP.NET Core:                5.0.0-preview.4.20218.5


PR

Starting PR run on '59215918e90c863b3e78e02417efc9c266de9077'...
| Description |     RPS | CPU (%) | Memory (MB) | Avg. Latency (ms) | Startup (ms) | Build Time (ms) | Published Size (KB) | First Request (ms) | Latency (ms) | Errors | Ratio |
| ----------- | ------- | ------- | ----------- | ----------------- | ------------ | --------------- | ------------------- | ------------------ | ------------ | ------ | ----- |
|      Before | 342,994 |      99 |          88 |              6.71 |          511 |           27023 |              120299 |             160.31 |         0.46 |      0 |  1.00 |
|       After | 346,571 |      99 |          88 |              6.72 |          487 |            7511 |              120299 |             130.97 |         0.78 |      0 |  1.01 |


@analogrelay analogrelay added this to the 5.0.0-preview5 milestone Apr 20, 2020
@benaadams benaadams force-pushed the ParseConnection-Test branch from 5921591 to d5166ab Compare April 23, 2020 04:01
@benaadams benaadams changed the title Test Make connection options parsing "safe" Apr 23, 2020
@benaadams
Copy link
Member Author

@aspnet-hello benchmark

@pr-benchmarks
Copy link

pr-benchmarks bot commented Apr 23, 2020

Starting 'Default' pipelined plaintext benchmark with session ID '6956c934428d4280ab7f200bf6ae765e'. This could take up to 30 minutes...

@pr-benchmarks
Copy link

pr-benchmarks bot commented Apr 23, 2020

Baseline

Starting baseline run on '38577e85ce7edee13c907c4bd778f2ca267e6ee8'...
RequestsPerSecond:           342,428
Max CPU (%):                 99
WorkingSet (MB):             90
Avg. Latency (ms):           6.8
Startup (ms):                503
First Request (ms):          153.82
Latency (ms):                0.64
Total Requests:              5,170,525
Duration: (ms)               15,100
Socket Errors:               0
Bad Responses:               0
Build Time (ms):             64,520
Published Size (KB):         120,355
SDK:                         5.0.100-preview.2.20120.3
Runtime:                     5.0.0-preview.5.20221.15
ASP.NET Core:                5.0.0-preview.5.20222.8


PR

Starting PR run on 'd5166aba42e657a00c3bc519ffb8f6ebb3ad993a'...
| Description |     RPS | CPU (%) | Memory (MB) | Avg. Latency (ms) | Startup (ms) | Build Time (ms) | Published Size (KB) | First Request (ms) | Latency (ms) | Errors | Ratio |
| ----------- | ------- | ------- | ----------- | ----------------- | ------------ | --------------- | ------------------- | ------------------ | ------------ | ------ | ----- |
|      Before | 342,428 |      99 |          90 |               6.8 |          503 |           64520 |              120355 |             153.82 |         0.64 |      0 |  1.00 |
|       After | 341,161 |      99 |          90 |              6.76 |          485 |            7502 |              120355 |              121.6 |         0.93 |      0 |  1.00 |


@benaadams
Copy link
Member Author

@aspnet-hello benchmark

@pr-benchmarks
Copy link

pr-benchmarks bot commented Apr 23, 2020

Starting 'Default' pipelined plaintext benchmark with session ID 'b27aa4bd605b42ec983ea44c9924b378'. This could take up to 30 minutes...

@pr-benchmarks
Copy link

pr-benchmarks bot commented Apr 23, 2020

Baseline

stdout: Starting baseline run on '38577e85ce7edee13c907c4bd778f2ca267e6ee8'...
The specified client url 'http://127.0.0.1:5002' is invalid or not responsive.


stderr: Baseline benchmark run on '38577e85ce7edee13c907c4bd778f2ca267e6ee8' failed.

PR


@benaadams
Copy link
Member Author

Raised issue for some redundant checks the Jit does dotnet/runtime#35348

@benaadams
Copy link
Member Author

@aspnet-hello benchmark

@pr-benchmarks
Copy link

pr-benchmarks bot commented Apr 23, 2020

Starting 'Default' pipelined plaintext benchmark with session ID 'e8fb684c82c34179afeff44d7354da37'. This could take up to 30 minutes...

@pr-benchmarks
Copy link

pr-benchmarks bot commented Apr 23, 2020

Baseline

Starting baseline run on '38577e85ce7edee13c907c4bd778f2ca267e6ee8'...
RequestsPerSecond:           352,614
Max CPU (%):                 99
WorkingSet (MB):             92
Avg. Latency (ms):           6.61
Startup (ms):                516
First Request (ms):          161.88
Latency (ms):                0.58
Total Requests:              5,324,289
Duration: (ms)               15,100
Socket Errors:               0
Bad Responses:               0
Build Time (ms):             27,019
Published Size (KB):         120,355
SDK:                         5.0.100-preview.2.20120.3
Runtime:                     5.0.0-preview.5.20221.15
ASP.NET Core:                5.0.0-preview.5.20223.1


PR

Starting PR run on '35b259753593be3e9a911b8d936348c0d4a38771'...
| Description |     RPS | CPU (%) | Memory (MB) | Avg. Latency (ms) | Startup (ms) | Build Time (ms) | Published Size (KB) | First Request (ms) | Latency (ms) | Errors | Ratio |
| ----------- | ------- | ------- | ----------- | ----------------- | ------------ | --------------- | ------------------- | ------------------ | ------------ | ------ | ----- |
|      Before | 352,614 |      99 |          92 |              6.61 |          516 |           27019 |              120355 |             161.88 |         0.58 |      0 |  1.00 |
|       After | 356,683 |      99 |          90 |              6.48 |          494 |            7506 |              120355 |             129.31 |         0.78 |      0 |  1.01 |


@benaadams benaadams marked this pull request as ready for review April 23, 2020 15:57
@benaadams
Copy link
Member Author

@aspnet-hello benchmark

@pr-benchmarks
Copy link

pr-benchmarks bot commented Apr 24, 2020

Starting 'Default' pipelined plaintext benchmark with session ID '93ad8b7cf01d4f43a926eede6f332a85'. This could take up to 30 minutes...

@pr-benchmarks
Copy link

pr-benchmarks bot commented Apr 24, 2020

Baseline

Starting baseline run on '38577e85ce7edee13c907c4bd778f2ca267e6ee8'...
RequestsPerSecond:           344,780
Max CPU (%):                 99
WorkingSet (MB):             86
Avg. Latency (ms):           6.68
Startup (ms):                487
First Request (ms):          157.28
Latency (ms):                0.41
Total Requests:              5,200,114
Duration: (ms)               15,080
Socket Errors:               0
Bad Responses:               0
Build Time (ms):             29,008
Published Size (KB):         120,383
SDK:                         5.0.100-preview.2.20120.3
Runtime:                     5.0.0-preview.5.20223.1
ASP.NET Core:                5.0.0-preview.5.20223.13


PR

Starting PR run on 'c0ddcd302263ac8159251f4c68a4d61ea3f5ca69'...
| Description |     RPS | CPU (%) | Memory (MB) | Avg. Latency (ms) | Startup (ms) | Build Time (ms) | Published Size (KB) | First Request (ms) | Latency (ms) | Errors | Ratio |
| ----------- | ------- | ------- | ----------- | ----------------- | ------------ | --------------- | ------------------- | ------------------ | ------------ | ------ | ----- |
|      Before | 344,780 |      99 |          86 |              6.68 |          487 |           29008 |              120383 |             157.28 |         0.41 |      0 |  1.00 |
|       After | 336,054 |      99 |          88 |              6.92 |          489 |            7503 |              120383 |             132.11 |         0.69 |      0 |  0.97 |


@benaadams benaadams force-pushed the ParseConnection-Test branch from c0ddcd3 to 74657de Compare April 24, 2020 23:38
@benaadams
Copy link
Member Author

@aspnet-hello benchmark

@pr-benchmarks
Copy link

pr-benchmarks bot commented Apr 25, 2020

Starting 'Default' pipelined plaintext benchmark with session ID '85ef06608bea4fd3864b6725e3401228'. This could take up to 30 minutes...

@pr-benchmarks
Copy link

pr-benchmarks bot commented Apr 25, 2020

Baseline

Starting baseline run on '3d034feef860002ff53ed70f56d7d11301763293'...
RequestsPerSecond:           346,907
Max CPU (%):                 99
WorkingSet (MB):             92
Avg. Latency (ms):           6.71
Startup (ms):                478
First Request (ms):          132.07
Latency (ms):                0.76
Total Requests:              5,236,275
Duration: (ms)               15,090
Socket Errors:               0
Bad Responses:               0
Build Time (ms):             10,004
Published Size (KB):         120,383
SDK:                         5.0.100-preview.2.20120.3
Runtime:                     5.0.0-preview.5.20223.1
ASP.NET Core:                5.0.0-preview.5.20223.13


PR

Starting PR run on 'd6eacd443c7e83dd11ffef129101bccc8027f054'...
| Description |     RPS | CPU (%) | Memory (MB) | Avg. Latency (ms) | Startup (ms) | Build Time (ms) | Published Size (KB) | First Request (ms) | Latency (ms) | Errors | Ratio |
| ----------- | ------- | ------- | ----------- | ----------------- | ------------ | --------------- | ------------------- | ------------------ | ------------ | ------ | ----- |
|      Before | 346,907 |      99 |          92 |              6.71 |          478 |           10004 |              120383 |             132.07 |         0.76 |      0 |  1.00 |
|       After | 349,939 |      99 |          90 |              6.61 |          511 |            7503 |              120383 |             151.09 |         0.55 |      0 |  1.01 |


@benaadams
Copy link
Member Author

@aspnet-hello benchmark

@pr-benchmarks
Copy link

pr-benchmarks bot commented Apr 25, 2020

Starting 'Default' pipelined plaintext benchmark with session ID 'd28ca040955242bbbe01601bb8678e74'. This could take up to 30 minutes...

@pr-benchmarks
Copy link

pr-benchmarks bot commented Apr 25, 2020

Baseline

Starting baseline run on '3d034feef860002ff53ed70f56d7d11301763293'...
RequestsPerSecond:           351,551
Max CPU (%):                 99
WorkingSet (MB):             90
Avg. Latency (ms):           6.64
Startup (ms):                474
First Request (ms):          169.11
Latency (ms):                0.54
Total Requests:              5,308,453
Duration: (ms)               15,100
Socket Errors:               0
Bad Responses:               0
Build Time (ms):             28,511
Published Size (KB):         120,383
SDK:                         5.0.100-preview.2.20120.3
Runtime:                     5.0.0-preview.5.20223.1
ASP.NET Core:                5.0.0-preview.5.20223.13


PR

Starting PR run on '0632ea1b812bfced5eadcd704f95d605ab266e97'...
| Description |     RPS | CPU (%) | Memory (MB) | Avg. Latency (ms) | Startup (ms) | Build Time (ms) | Published Size (KB) | First Request (ms) | Latency (ms) | Errors | Ratio |
| ----------- | ------- | ------- | ----------- | ----------------- | ------------ | --------------- | ------------------- | ------------------ | ------------ | ------ | ----- |
|      Before | 351,551 |      99 |          90 |              6.64 |          474 |           28511 |              120383 |             169.11 |         0.54 |      0 |  1.00 |
|       After | 352,921 |      99 |          92 |              6.57 |          477 |            7504 |              120383 |             128.48 |          0.9 |      0 |  1.00 |


@benaadams
Copy link
Member Author

Tried resolving conflicts via GitHub UI, lets see how it goes :)

@benaadams
Copy link
Member Author

Looked like it worked :)

@benaadams
Copy link
Member Author

@aspnet-hello benchmark

@pr-benchmarks
Copy link

pr-benchmarks bot commented Apr 28, 2020

Starting 'Default' pipelined plaintext benchmark with session ID '684b66427d404f2097f57425616ea12e'. This could take up to 30 minutes...

@pr-benchmarks
Copy link

pr-benchmarks bot commented Apr 28, 2020

Baseline

Starting baseline run on '5fc067266f739695291acc935ae0dd6a4a7a8e78'...
RequestsPerSecond:           342,290
Max CPU (%):                 99
WorkingSet (MB):             89
Avg. Latency (ms):           6.79
Startup (ms):                562
First Request (ms):          170.16
Latency (ms):                0.47
Total Requests:              5,168,158
Duration: (ms)               15,100
Socket Errors:               0
Bad Responses:               0
Build Time (ms):             25,011
Published Size (KB):         120,463
SDK:                         5.0.100-preview.2.20120.3
Runtime:                     5.0.0-preview.5.20225.8
ASP.NET Core:                5.0.0-preview.5.20223.13


PR

Starting PR run on '78281ddd973b86af94788265c934e4de06558d1f'...
| Description |     RPS | CPU (%) | Memory (MB) | Avg. Latency (ms) | Startup (ms) | Build Time (ms) | Published Size (KB) | First Request (ms) | Latency (ms) | Errors | Ratio |
| ----------- | ------- | ------- | ----------- | ----------------- | ------------ | --------------- | ------------------- | ------------------ | ------------ | ------ | ----- |
|      Before | 342,290 |      99 |          89 |              6.79 |          562 |           25011 |              120463 |             170.16 |         0.47 |      0 |  1.00 |
|       After | 353,649 |      99 |          88 |              6.49 |          489 |            7502 |              120463 |             132.18 |         1.02 |      0 |  1.03 |


@halter73 halter73 merged commit 3af92e2 into dotnet:master Apr 29, 2020
@halter73
Copy link
Member

Thanks again!

@benaadams benaadams deleted the ParseConnection-Test branch May 27, 2020 06:47
@amcasey amcasey added area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions and removed area-runtime labels Jun 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants