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
Cronet fixes #20464
Cronet fixes #20464
Conversation
rmstar
commented
Oct 3, 2019
•
edited
edited
- Fixed bug in cronet_transport where completed ops were not removed from storage
- Don't collect cronet netlog. It's slow and file size can be huge
- Cronet Perf tests: Added 1 KB unary RPC test case
- Reduce number of messages from 10000->1000 to reduce test run time.
@@ -30,6 +30,5 @@ void configureCronet(void) { | |||
inDomains:NSUserDomainMask] lastObject]; | |||
NSLog(@"Documents directory: %@", url); | |||
[Cronet start]; | |||
[Cronet startNetLogToFile:@"cronet_netlog.json" logBytes:YES]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of removing it, maybe make this configurable? Sometimes netlog is useful.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed.
@@ -181,7 +181,7 @@ - (void)testPingPongRPCWithV2API { | |||
[self pingPongV2APIWithRequest:request numMessages:1000 options:options]; | |||
|
|||
[self measureBlock:^{ | |||
[self pingPongV2APIWithRequest:request numMessages:10000 options:options]; | |||
[self pingPongV2APIWithRequest:request numMessages:1000 options:options]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you explain a bit why we are reducing the number? Maybe in the PR description. For record.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's to reduce test run time. Takes too long with 10k messages, and we don't really get any additional benefit with more messages. I've updated the PR description.