Performance benchmarks can be found below. Obviously, a 8 KiB
request/response is tailored to showcase this improvement as this is
where codec buffer reuse shines, but I've run other benchmarks too (like
1-byte requests and responses) and there's no discernable impact on
performance.
We do not allow reuse of buffers when stat handlers or binlogs are
turned on. This is because those two may need access to the data and
payload even after the data has been written to the wire. In such cases,
we never return the data back to the pool.
A buffer reuse threshold of 1 KiB was determined after several
experiments. There's diminished returns when buffer reuse is enabled for
smaller messages (actually, a negative impact).
unary-networkMode_none-bufConn_false-keepalive_false-benchTime_40s-trace_false-latency_0s-kbps_0-MTU_0-maxConcurrentCalls_6-reqSize_8192B-respSize_8192B-compressor_off-channelz_false-preloader_false
Title Before After Percentage
TotalOps 839638 906223 7.93%
SendOps 0 0 NaN%
RecvOps 0 0 NaN%
Bytes/op 103788.29 80592.47 -22.35%
Allocs/op 183.33 189.30 3.27%
ReqT/op 1375662899.20 1484755763.20 7.93%
RespT/op 1375662899.20 1484755763.20 7.93%
50th-Lat 238.746µs 225.019µs -5.75%
90th-Lat 514.253µs 456.439µs -11.24%
99th-Lat 711.083µs 702.466µs -1.21%
Avg-Lat 285.45µs 264.456µs -7.35%
Simplified the tests by only testing what is required and faking out
whatever can be faked out.
Also added a fakexds.Server implementation. Will switch other users of
the existing fakeserver implementation after this PR is merged.
Changes to "xds_server" field in bootstrap file:
1. Field name is changed "xds_servers" (plural).
2. Field value should be a list of objects instead of a single object.
For now, we can ignore all entries except the first one. In the future, we will add support for falling back to a secondary server when the primary is not reachable.
The real test relies on req chan to know when the fake xds server
receives the request. Without this, req chan may unblock for the ack,
before the real watch handler is ready.
Although the existing code wasn't causing a deadlock or causing
goroutines to hang forever, this is cleaner and prevents any such thing
from happening in the future.
Small refactor of tests:
- Add a channel to remoteBalancer on which to send received stats messages
- Change runAndGetStats to runAndCheckStats to allow for faster successful test
runs (no longer need to sleep for one whole second per test).
- Remove redundant leak check from runAndCheckStats, otherwise excess load
report messages not read from the channel will result in an infinite loop.
- Add String method to rpcStats to avoid data race between merging and printing.
The client will send a request with version/nonce after receiving a
response, to ack/nack.
Ack versions for different xds types are independent.
Some other changes
- merge sendRequests to one shared function, with fields for version/nonce
- deleted enum for xds types, and always use const URL string
Errors will be handled specifically, depending on whether it's a
connection error or other types of errors.
Without this fix, balancer's callback will be called with <nil> update,
causing nil panic later.
- install cds balancer by importing package cdsbalancer
- `net.SplitHostPort` fails when target doesn't have port, but we want to use the original string instead
This PR also adds a new testutils directory with a fake client and a
channel which supports a timed receive operation. In follow up PRs, we
can move other common test stuff like the fake server etc to this
directory and cleanup more tests.
The client will look only at the last route in the list (the default
route), whose match field must contain a prefix field whose value is the
empty string and whose route field must be set.
Continuing the war on stacks, we can reduce the amount of stack required
per-RPC by combining defers from different components into one.
Each defer statement in process{Unary,Streaming}RPC goes on the stack
and occupies about 56-64 bytes the entire lifetime of an RPC, which
could be very long. More importantly, a call to runtime.morestack is
often required to allocate a new, larger stack when the handler
goroutine runs out of stack memory (Go's default stack size is 2 KiB).
Before:
$ go tool objdump <binary> | grep "TEXT.*processUnaryRPC(SB)" -A 10 | grep "SUBQ.*SP"
server.go:867 0x9132fb 4881ec80030000 SUBQ $0x380, SP
$ go tool objdump <binary> | grep "TEXT.*processStreamingRPC(SB)" -A 10 | grep "SUBQ.*SP"
server.go:1099 0x9151bb 4881ec68020000 SUBQ $0x268, SP
After:
$ go tool objdump <binary> | grep "TEXT.*processUnaryRPC(SB)" -A 10 | grep "SUBQ.*SP"
server.go:867 0x9132fb 4881ecd0020000 SUBQ $0x2d0, SP
$ go tool objdump <binary> | grep "TEXT.*processStreamingRPC(SB)" -A 10 | grep "SUBQ.*SP"
server.go:1116 0x9150fb 4881ecf8010000 SUBQ $0x1f8, SP
As one can observe, the processUnaryRPC's stack goes down from 0x380
bytes to 0x2d0 bytes (896 - 720 = 176 bytes) while processStreamingRPC's
stack goes down from 0x2d8 bytes to 0x1f8 bytes (616 - 504 = 112 bytes).
There are probably other things we can do here, but these are some low
hanging fruits to pick off.
This PR removes the xds_client implementation from eds balancer, and replaces it with a xds_client wrapper. (The xds_client wrapper has very similar API as the old xds_client implementation, so the change in the eds balancer is minimal).
The eds balancer currently doesn't look for xds_client from attributes, and always creates a new xds_client. The attributes change will be done in a following up change.
* Add a helper to the fakexds package to return a ClientConn talking to
the fake server.
* Tests will make use of this ClientConn wherever required, or they will
directly pass the fake server's address as the balancerName to the
xdsclient.New() function, thus exercising that code path as well.
* Add grpc.WithTimeout to list in vet.sh
