Skip to content

Conversation

ZhouyihaiDing
Copy link
Contributor

@ZhouyihaiDing ZhouyihaiDing commented Jul 6, 2017

  1. Add a file benchmain/main.go to run the benchmark using go run main.go -flag=xxx.

In benchmark17_test.go, it uses "b *testing.B" to calculate the allocations, and b.N to stop benchmarks. In main.go, it uses ReadMemStats and do the calculation by it's own and timeout to stop benchmarks.

An example of main.go is
go run benchmark/benchmain/main.go -kbps=0 -mtu=0 -maxConcurrentCalls=1 -reqSizeBytes=1,1048576 -reqspSizeBytes=1,1048576 -runUnary=true -runStream=true -traceMode=true -latency=0s,5ms -timeout=10s
Flags include

-runUnary                        =bool
-runStream                       =bool
-traceMode                       =bool
-compressionMode                 =bool
-readLatency                     =slice of time.Duration like 0s,10ms
-readKbps                        =slice of int like 0,128,512
-readMtu                         =slice of int
-readMaxConcurrentCalls          =slice of int
-readReqSizeBytes                =slice of int
-readReqspSizeBytes              =slice of int
-timeout                         =time.Duration like 10s

runUnary and runStream can be set both. traceMode and compressionMode are false by default.
All flags have default settings.

  1. Add profile flags
-cpuProfile       = string of filename
-memProfile       = string of filename
-memProfileRate   = int

when cpuProfile or memProfile is set, it will save the profile result into the file.

@dfawley dfawley requested a review from MakMukhi July 7, 2017 20:40
@dfawley dfawley assigned ZhouyihaiDing and unassigned MakMukhi Jul 21, 2017
@dfawley
Copy link
Member

dfawley commented Jul 21, 2017

Please resolve the conflicts and let us know when this is ready to review again. Sorry, thanks.

@dfawley dfawley assigned MakMukhi and unassigned ZhouyihaiDing Aug 4, 2017
@ZhouyihaiDing ZhouyihaiDing force-pushed the benchmain branch 2 times, most recently from bb5a09a to 0619d4e Compare August 8, 2017 20:26
Copy link
Contributor

@MakMukhi MakMukhi left a comment

Choose a reason for hiding this comment

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

Looks good, minor suggestions

}),
)
tc := testpb.NewBenchmarkServiceClient(conn)
return func(pos int) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is pos needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In the runStream, it need the position to indicate which steam is using.
Since runUnary and runStream shares the same function runBenchmark, runUnary set a postion without using it.

// Initiate main function to get settings of features.
func init() {
var runUnary, runStream bool
var traceMode, noTraceMode bool
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we have only one variable traceMode with default value to be false?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done. I do the same with runCompressionMode. false by default, true if set.

*values = replace
}

func readTimeFromIntSlice(values *[]time.Duration, replace string) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we change the name of this function since it isn't really reading from an intSlice?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done. I don't know which name is better, so I change it from IntSlice to Input.
Ready for review.

@MakMukhi MakMukhi assigned ZhouyihaiDing and unassigned MakMukhi Aug 17, 2017
target, stopper := bm.StartServer(bm.ServerInfo{Addr: "localhost:0", Type: "protobuf", Network: nw}, sopts...)
conn := bm.NewClientConn(target, opts...)
tc := testpb.NewBenchmarkServiceClient(conn)
return func(pos int) {
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: This could have have been an unnamed variable since this is not used in the function and is there only to maintain function signature.

return func(_ int) {
...
}

Copy link
Member

Choose a reason for hiding this comment

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

If all your parameters are unnamed, you can do this instead:

return func(int) {
...
}

@MakMukhi MakMukhi assigned ZhouyihaiDing and unassigned MakMukhi Aug 22, 2017
@MakMukhi MakMukhi assigned ZhouyihaiDing and unassigned MakMukhi Aug 23, 2017
@dfawley dfawley merged commit 7db1564 into grpc:master Aug 24, 2017
@dfawley dfawley modified the milestone: 1.6 Release Aug 28, 2017
menghanl pushed a commit to menghanl/grpc-go that referenced this pull request Aug 30, 2017
@lock lock bot locked as resolved and limited conversation to collaborators Jan 18, 2019
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.

4 participants