fortio: optimized http client, simplified logging #470

ldemailly · 2017-07-16T07:57:17Z

On my Mac:
Old net/http : 19.7k qps with 2 threads (-http -1)
New -http 1 : 30.5k qps with 2 threads
And 1/8th of the user cpu consumed

Interface to select between the 2 clients

Edit:
Now works with both chunked and fixed
Content-Length (like echosrv)

Still too many ifs / readResponse function is too messy

(was intially stacked on top of #460, rerooted from master later)

This change is

- build and use fortio in perf/standalone scripts - (win) fix for having more connections than requested

Also changing threshold for sleep time histogram to 5%

Now getting exactly the expected number of sockets But performance is still significantly lower than wrk

On my Mac: Old net/http : 19.7k qps with 2 threads (-http -1) New -http 1 : 30.5k ops with 2 threads And 1/8th of the user cpu Interface to select between the 2 clients Wip because it doesn’t work for chunked yet only for fixed Content-Length (like echosrv)

ran gazelle

ldemailly · 2017-07-16T09:44:53Z

on the benchmark boxes:
40->60->100k qps compared to master, #460, #470

fortio with new http client:

ldemailly@benchmark-1:~/go/src/istio.io/istio$ time fortio -http 1 -qps 0 -t 20s -c 16 -r 0.0001 -gomaxprocs 16 http://benchmark-2:8080/
Fortio running at 0 queries per second, 4->16 procs, for 20s: http://benchmark-2:8080/
Starting at max qps with 16 thread(s) [gomax 16] for 20s
Ended after 20.000451762s : 2084445 calls. qps=1.0422e+05
Aggregated Function Time : count 2084445 avg 0.00015328816 +/- 5.803e-05 min 6.7947e-05 max 0.005139873 sum 319.520748
# range, mid point, percentile, count
< 0.0001 , 0.0001 , 3.38, 70486
>= 0.0001 < 0.0002 , 0.00015 , 90.84, 1823090
>= 0.0002 < 0.0003 , 0.00025 , 97.13, 131029
>= 0.0003 < 0.0004 , 0.00035 , 98.82, 35296
>= 0.0004 < 0.0005 , 0.00045 , 99.71, 18561
>= 0.0005 < 0.0006 , 0.00055 , 99.90, 3902
>= 0.0006 < 0.0007 , 0.00065 , 99.95, 1011
>= 0.0007 < 0.0008 , 0.00075 , 99.98, 560
>= 0.0008 < 0.0009 , 0.00085 , 99.99, 294
>= 0.0009 < 0.001 , 0.00095 , 99.99, 89
>= 0.001 < 0.0011 , 0.00105 , 100.00, 52
>= 0.0011 < 0.0012 , 0.00115 , 100.00, 11
>= 0.0012 < 0.0014 , 0.0013 , 100.00, 18
>= 0.0014 < 0.0016 , 0.0015 , 100.00, 1
>= 0.0016 < 0.0018 , 0.0017 , 100.00, 1
>= 0.0018 < 0.002 , 0.0019 , 100.00, 18
>= 0.002 < 0.0025 , 0.00225 , 100.00, 3
>= 0.0025 < 0.003 , 0.00275 , 100.00, 4
>= 0.003 < 0.0035 , 0.00325 , 100.00, 1
>= 0.0035 < 0.004 , 0.00375 , 100.00, 2
>= 0.0045 < 0.005 , 0.00475 , 100.00, 6
>= 0.005 < 0.006 , 0.0055 , 100.00, 10
# target 50% 0.000153302
# target 75% 0.000181886
# target 99% 0.000419932
# target 99.9% 0.000599912
Code 200 : 2084445
Response Header Sizes : count 2084445 avg 116 +/- 0 min 116 max 116 sum 241795620
Response Body/Total Sizes : count 2084445 avg 116 +/- 0 min 116 max 116 sum 241795620

real	0m20.059s
user	0m14.668s
sys	0m36.340s

wrk:

ldemailly@benchmark-1:~/go/src/istio.io/istio$ time ~/github/wrk/wrk -d 20 -c 16 -t 16 --latency http://benchmark-2:8080/
Running 20s test @ http://benchmark-2:8080/
  16 threads and 16 connections
  Thread Stats   Avg      Stdev     Max   +/- Stdev
    Latency   145.50us  132.29us  12.75ms   95.64%
    Req/Sec     7.19k   264.29     8.17k    72.33%
  Latency Distribution
     50%  127.00us
     75%  149.00us
     90%  182.00us
     99%  461.00us
  2301291 requests in 20.10s, 254.58MB read
Requests/sec: 114492.59
Transfer/sec:     12.67MB

real	0m20.147s
user	0m5.492s
sys	0m35.184s

new profile:

(pprof) top15
30670ms of 34870ms total (87.96%)
Dropped 51 nodes (cum <= 174.35ms)
Showing top 15 nodes out of 55 (cum >= 530ms)
      flat  flat%   sum%        cum   cum%
   14150ms 40.58% 40.58%    15130ms 43.39%  syscall.Syscall
    7290ms 20.91% 61.49%     7290ms 20.91%  runtime.futex
    2080ms  5.97% 67.45%     2080ms  5.97%  runtime.epollwait
    1400ms  4.01% 71.47%    13230ms 37.94%  runtime.findrunnable
    1060ms  3.04% 74.51%     1060ms  3.04%  istio.io/istio/devel/fortio.FoldFind
     960ms  2.75% 77.26%    18630ms 53.43%  istio.io/istio/devel/fortio.(*BasicClient).Fetch
     860ms  2.47% 79.72%      860ms  2.47%  runtime.usleep
     770ms  2.21% 81.93%      770ms  2.21%  runtime.runqgrab
     400ms  1.15% 83.08%     1170ms  3.36%  runtime.runqsteal
     380ms  1.09% 84.17%      380ms  1.09%  runtime.casgstatus
     310ms  0.89% 85.06%      450ms  1.29%  runtime.lock
     270ms  0.77% 85.83%      270ms  0.77%  istio.io/istio/devel/fortio.(*Histogram).Record
     260ms  0.75% 86.58%    19240ms 55.18%  main.test
     250ms  0.72% 87.30%      250ms  0.72%  runtime._ExternalCode
     230ms  0.66% 87.96%      530ms  1.52%  runtime.reentersyscall

douglas-reid · 2017-07-16T15:42:02Z

devel/fortio/http.go

+
+const toLowerMask byte = ('a' - 'A')
+
+// ASCIIFold returns a byte array equal to the input string but in lowercase.


any reason to not use something like: byte[](strings.ToLower(str)) ?

yes it's slower/doing unicode correctness (will add a benchmark) and tests the folding. though for since function might be simpler

also thanks for having looked but this one is still wip, #460 is ready though!

BenchmarkASCIIFoldNormalToLower-8 3000000 415 ns/op 96 B/op 3 allocs/op BenchmarkASCIIFoldCustomToLowerMap-8 5000000 362 ns/op 96 B/op 3 allocs/op BenchmarkASCIIFold-8 10000000 116 ns/op 32 B/op 1 allocs/op

So it's 3.5x faster and 1/3rd of memory (this being said it doesn't matter too much given the absolute numbers) - I'm also changing the code to upper case to not mangle \r\n etc

Folding is 4x faster than built in, 1/3rd of allocs Search is 10x faster, no alloc instead of a lot ``` BenchmarkASCIIFoldNormalToLower-8 3000000 416 ns/op 96 B/op 3 allocs/op BenchmarkASCIIFoldCustomToLowerMap-8 5000000 371 ns/op 96 B/op 3 allocs/op BenchmarkASCIIToUpper-8 20000000 104 ns/op 32 B/op 1 allocs/op BenchmarkFoldFind0-8 200000 7621 ns/op 2112 B/op 3 allocs/op BenchmarkFoldFind-8 2000000 669 ns/op 0 B/op 0 allocs/op ```

unit test for parsing, reads the first chunk

The granularity seems not great no ?

- added some doc in WORKSPACE about wool - initial support for fasthttp

douglas-reid · 2017-07-25T18:36:09Z

devel/fortio/README.md

+    	Enable http compression
+  -gomaxprocs int
+    	Setting for runtime.GOMAXPROCS, <1 doesn't change the default
+  -http int


a small nit: I'd prefer if this were not integer values.

something like:

--http --fast_http1 --fast_http1.1 --fast_http1.1_no_keep_alive

where it defaults to --http

changing/splitting flags

douglas-reid · 2017-07-25T18:45:02Z

devel/fortio/cmd/echosrv/echo.go

 	"net/http"
 	"sync/atomic"

-	"github.com/golang/glog"
+	log "istio.io/istio/devel/fortio"


do we need/want this alias? any harm in just having the calls be fortio.LogV() and fortio.DbgOn() ?

log.DbgOn() was shorter and clearer that it's about logging (I guess this stems from the fact that in theory it should be it's own package - or better, not reinvented - but fortio is sort of a bundle of features that can be used as a whole or piecemeal)

my objection, if you will, here is that you are aliasing a package (that you control) for no reason other than to shorten it and, as a result, we end up with a bit of stutter (log.LogV()).

it would be nice if it were a distinct package, but I don't know that it is required.

I notice elsewhere that we don't have the same alias. If anything, I would suggest a consistent approach.

douglas-reid · 2017-07-25T18:50:20Z

devel/fortio/cmd/fortio/main.go

@@ -87,13 +83,12 @@ func main() {
 		compressionFlag = flag.Bool("compression", false, "Enable http compression")
 		goMaxProcsFlag  = flag.Int("gomaxprocs", 0, "Setting for runtime.GOMAXPROCS, <1 doesn't change the default")
 		profileFlag     = flag.String("profile", "", "write .cpu and .mem profiles to file")
-		headersFlags    flagList
+		httpClient      = flag.Int("http", 1,


i commented on this above, but some more food for thought: generally, for api design, we start with 0 as the default and move up. So, I wouldn't expect -1 to be a valid value here.

Something like the following is more what I would expect:

enum HttpClient { HTTP_CLIENT_UNSPECIFIED = 0; FAST_1_0 = 1; FAST_1_1 = 2; FAST_1_1_NO_KEEP_ALIVE = 3; }

in go, this might be:

type HttpClient int const ( Standard HttpClient = iota Fast Fast11 Fast11NoKeepAlive )

to use in a flag, you might have to make HttpClient implement flag.Value for this approach however.

this one is only in the main, so it's not really an api design but just how to expediently select which client and option to use, I do agree strings would be cleaner and an auto mapping from string<->enum like I implemented in logger.go is nicer but it's also quite a lot more code - I will make some change, thanks!

douglas-reid · 2017-07-25T19:10:48Z

devel/fortio/http.go

+	// Host is treated specially, remember that one separately.
+	hostOverride string
+	// Buffer size for optimized client
+	bufferSizeKbFlag                = flag.Int("httpbufferkb", 32, "Size of the buffer (max data size) for the optimized http client in kbytes")


typically, flags are placed in the main code, and not in the library code. any reason to have them here instead?

it's sort of a constant, but configurable, but I can move it in the constructor but it'll more more hassles to transmit it

Another option: export the var, and in the cmd, use something like flag.IntVar to reference the variable.

ok: I moved those 2 flags (in theory I should change the error messages which also assumed the fix was under -httpbufferkb)

douglas-reid · 2017-07-25T19:19:57Z

devel/fortio/http.go

+			bc.req = append(bc.req, []byte("Connection: close\r\n")...)
+		}
+	}
+	for h := range extraHeaders {


did you look at bytes.Buffer for usage here? Instead of all the casting and appends, etc., you could have buf.Write() and buf.WriteString() calls ?

thx ! I'll change and remove the // TODO: ugly ... what's a good/elegant and efficient way to do this

douglas-reid · 2017-07-25T19:28:33Z

devel/fortio/logger.go

+}
+
+// LogOn returns true if a given level is currently logged.
+func LogOn(lvl LogLevel) bool {


nit: i'd just call this LogEnabled

my first version was IsLogEnabled() and IsLoggingDebug() but I tried to go "go way" (shortest readable)

So, we definitely don't want the Is prefix. I don't personally like On, but I won't object if that is your preference.

👍 I do prefer Enabled but it's long and feels unnatural in go

douglas-reid · 2017-07-25T19:29:00Z

devel/fortio/logger.go

+
+// Log at the given level.
+// 2 level of calls so it's always same depth for extracting caller file/line
+func Log(lvl LogLevel, format string, rest ...interface{}) {


if this takes a format string, would it be more appropriate to call it Logf ?

last PR you made me remove the F in my Print - and I agreed it was nicer :-)
but the main reason I'm trying to make the Api as short yet meaningful

I fully reserve the right to disagree with myself ;).

I don't remember the prior context, but just suggesting that appending f might make it clearer that there is formatting happening. Up to you.

ok I'm adding the "f" as that solves my variable vs function namespace conflict issue - thx!

douglas-reid · 2017-07-25T19:29:16Z

devel/fortio/logger.go

+// -- would be nice to be able to create those in a loop instead of copypasta:
+
+// Dbg logs if Debug level is on.
+func Dbg(format string, rest ...interface{}) {


nit: expand to Debugf

can I keep Dbg :-) ?

Sure. I'm not going to block the PR over it, but I think Dbg might be a bit too short (and harder to guess for non-ldemailly users of the API). Debugf is easier to anticipate.

changed !
also thanks to adding the f I changed the bool LogOn() -> Log() and
DebugOn() -> LogDebug()

if LogDebug() {

flows nicely

douglas-reid · 2017-07-25T19:30:06Z

devel/fortio/logger.go

+}
+
+// Info logs if Info level is on.
+func Info(format string, rest ...interface{}) {


here and elsewhere, if format strings are taken, it seems like using Infof might be clearer.

only weird one now is LogVf() is quite a mouthful

douglas-reid · 2017-07-25T19:30:38Z

devel/fortio/logger.go

+}
+
+// DbgOn is a shortcut for LogOn(D).
+func DbgOn() bool {


again, small issue, but I prefer DebugEnabled

Wasn’t working with last chunk (0\r\n\r\n) being in a separate frame

ldemailly · 2017-07-26T00:25:10Z

Thanks a lot for all the notes. I made inline replies and changes PTAL!

douglas-reid · 2017-07-26T17:21:48Z

devel/fortio/logger.go

+}
+
+// VOn is a shortcut for LogOn(V).
+func VOn() bool {


here, you might consider just V(). that would more closely align with glog and expectations (though it notably does not take an arg).

however, i'm a bit worried about the difference in VOn() and DbgOn(). It feels like it should either be VOn() and DOn() or VerboseOn() and DebugOn (if we go with fuller const names).

it's DbgOn() because the logging shortcut is Dbg()
V() was a bit short for a function and Verbose() a bit long and Verb() or Vrb() not clear or misleading so I called the shortcut for V level logging LogV()
so I guess this should in theory be LogVOn() or IsLoggingV()... not sure... naming is hard :-) suggestions welcome

ayj · 2017-07-26T19:02:27Z

Maybe this was answered elsewhere, but what is the rationale for creating a new logging package vs. using an existing one, e.g. glog?

thx doug!

ldemailly · 2017-07-26T19:46:27Z

Maybe this was answered elsewhere, but what is the rationale for creating a new logging package vs. using an existing one, e.g. glog?

A combination of things but in truth none great

learning opportunity for me
less dependencies/more self contained
glog logs verbose using "I" and doesn't have crit level
I prefer having Debug and Verbose below Info instead of numerical V(1) V(2)...
glog create files by default which I (nobody?) wants - everywhere I see the default changed
I like my api/names better :-)
optics :-)
opinionated :-)

ayj · 2017-07-26T20:58:22Z

learning opportunity for me
optics :-)
opinionated :-)

Does this set a precedent for other contributors re: NIH?

less dependencies/more self contained

glog doesn't have any non-stdlib dependencies and I suspect most of its stdlib dependencies are used elsewhere in any common golang program. Dependencies aren't necessarily a bad thing, particularly if they are from stdlib or another well-tested package.

glog logs verbose using "I" and doesn't have crit level

IMO the more named log levels, the less semantically meaningful they become. WARNING/ERROR/FATAL seems sufficient for non-info/debug related logs, e.g. WARNING = "might be bad", ERROR = "definitely bad but we can recover", FATAL = "bad+panic". I'm not sure where CRITICAL fits into that picture.

glog create files by default which I (nobody?) wants - everywhere I see the default changed

Agreed on this point though the default flag value can be overridden per binary with a one line change, e.g.

func main() {
    flag.Lookup("logtostderr").Value.Set("true")            // change default logging behavior
    flag.Parse()
    glog.Info("Hello, world")
}

I like my api/names better :-)

This seems like an argument against writing another logging package re: users familiarity with existing API/names :)

ldemailly · 2017-07-26T21:54:22Z

tests/e2e/tests/mixer/mixer_test.go

@@ -155,7 +155,7 @@ func (p *promProxy) Teardown() (err error) {
 	if p.portFwdProcess != nil {
 		err := p.portFwdProcess.Kill()
 		if err != nil {
-			glog.Error("Failed to kill port-forward process, pid: %s", p.portFwdProcess.Pid)
+			glog.Errorf("Failed to kill port-forward process, pid: %d", p.portFwdProcess.Pid)


2 bugs here found by linter, not sure how come it wasn't found by presubmit

douglas-reid

Looks pretty good to me. Thanks for your patience on my nits. The only thing I see that you may or may not care about are places where Log(Debug) could be written as LogDebug().

douglas-reid · 2017-07-27T16:04:17Z

devel/fortio/periodic.go

 		if percentNegative > 5 {
 			sleepTime.Print(os.Stdout, "Aggregated Sleep Time", 50)
 			fmt.Printf("WARNING %.2f%% of sleep were falling behind\n", percentNegative)
 		} else {
-			if Verbosity > 0 {
+			if Log(Verbose) {


could be LogVerbose() right?

douglas-reid · 2017-07-27T16:04:54Z

devel/fortio/periodic.go

 		funcTimes.Log(tIDStr+" Function duration", 99)
-		if verbosity > 2 {
+		if Log(Debug) {


here and elsewhere, could we use the LogDebug() helper method you added?

in some intermediate version I had completely deleted the shortcuts as Log(Verbose) is as short and clear (and more generic) as LogVerbose() until I got to the main packages where there one need to use fortio.Verbose, so I put them back but only for outside

ldemailly · 2017-07-27T16:52:49Z

thanks a lot for your time and your patience continuing my go education !

* fortio update - build and use fortio in perf/standalone scripts - fix for having more connections than requested * Updating sample to test using echosrv (slightly lower latency) Also changing threshold for sleep time histogram to 5% * Adding -gomaxprocs flag * One client per goroutine Now getting exactly the expected number of sockets But performance is still significantly lower than wrk * Adding profiler option * Homegrown http client On my Mac: Old net/http : 19.7k qps with 2 threads (-http -1) New -http 1 : 30.5k ops with 2 threads And 1/8th of the user cpu Interface to select between the 2 clients * More comments and backtrack test * http_test was missing from bazel ran gazelle * Added benchmark and fix folding bugs Folding is 4x faster than built in, 1/3rd of allocs Search is 10x faster, no alloc instead of a lot ``` BenchmarkASCIIFoldNormalToLower-8 3000000 416 ns/op 96 B/op 3 allocs/op BenchmarkASCIIFoldCustomToLowerMap-8 5000000 371 ns/op 96 B/op 3 allocs/op BenchmarkASCIIToUpper-8 20000000 104 ns/op 32 B/op 1 allocs/op BenchmarkFoldFind0-8 200000 7621 ns/op 2112 B/op 3 allocs/op BenchmarkFoldFind-8 2000000 669 ns/op 0 B/op 0 allocs/op ``` * chunked decoding unit test for parsing, reads the first chunk * fasthttp client wrapper (-http 3) - added some doc in WORKSPACE about wtool - initial support for fasthttp * Make linter happy though I might drop fasthttp as it’s not faster (but it’s more complete) and adds a lot of dependencies * Split connect out * Fixing merge error * adding fortio Logger * Fixing bazel build And misc comment/leftover from #460 etc * Shorter logger names * Switched to new logging * Eradicate Verbosity leftovers * Added -logprefix and -logcaller; Refactor start on http.go also fixed a couple of calls missing % codes * Minor edits, saving to GitHub wip * Woot http1.1 works now Still unwieldy but working ! * Some profiler optimizations * More profiler opt * Make lint happy by making checking connection closed header a flag * Dropping fasthttp dependency * Moved 3 changes to #495 * Minor: use version in echoers, use 3 digits * Code review updates (thx doug!) + serious chunked bug fix Wasn’t working with last chunk (0\r\n\r\n) being in a separate frame * Code review comment thx doug! Former-commit-id: e09dcaf

* fortio update - build and use fortio in perf/standalone scripts - fix for having more connections than requested * Updating sample to test using echosrv (slightly lower latency) Also changing threshold for sleep time histogram to 5% * Adding -gomaxprocs flag * One client per goroutine Now getting exactly the expected number of sockets But performance is still significantly lower than wrk * Adding profiler option * Homegrown http client On my Mac: Old net/http : 19.7k qps with 2 threads (-http -1) New -http 1 : 30.5k ops with 2 threads And 1/8th of the user cpu Interface to select between the 2 clients * More comments and backtrack test * http_test was missing from bazel ran gazelle * Added benchmark and fix folding bugs Folding is 4x faster than built in, 1/3rd of allocs Search is 10x faster, no alloc instead of a lot ``` BenchmarkASCIIFoldNormalToLower-8 3000000 416 ns/op 96 B/op 3 allocs/op BenchmarkASCIIFoldCustomToLowerMap-8 5000000 371 ns/op 96 B/op 3 allocs/op BenchmarkASCIIToUpper-8 20000000 104 ns/op 32 B/op 1 allocs/op BenchmarkFoldFind0-8 200000 7621 ns/op 2112 B/op 3 allocs/op BenchmarkFoldFind-8 2000000 669 ns/op 0 B/op 0 allocs/op ``` * chunked decoding unit test for parsing, reads the first chunk * fasthttp client wrapper (-http 3) - added some doc in WORKSPACE about wtool - initial support for fasthttp * Make linter happy though I might drop fasthttp as it’s not faster (but it’s more complete) and adds a lot of dependencies * Split connect out * Fixing merge error * adding fortio Logger * Fixing bazel build And misc comment/leftover from istio#460 etc * Shorter logger names * Switched to new logging * Eradicate Verbosity leftovers * Added -logprefix and -logcaller; Refactor start on http.go also fixed a couple of calls missing % codes * Minor edits, saving to GitHub wip * Woot http1.1 works now Still unwieldy but working ! * Some profiler optimizations * More profiler opt * Make lint happy by making checking connection closed header a flag * Dropping fasthttp dependency * Moved 3 changes to istio#495 * Minor: use version in echoers, use 3 digits * Code review updates (thx doug!) + serious chunked bug fix Wasn’t working with last chunk (0\r\n\r\n) being in a separate frame * Code review comment thx doug! Former-commit-id: e09dcaf

* fortio update - build and use fortio in perf/standalone scripts - fix for having more connections than requested * Updating sample to test using echosrv (slightly lower latency) Also changing threshold for sleep time histogram to 5% * Adding -gomaxprocs flag * One client per goroutine Now getting exactly the expected number of sockets But performance is still significantly lower than wrk * Adding profiler option * Homegrown http client On my Mac: Old net/http : 19.7k qps with 2 threads (-http -1) New -http 1 : 30.5k ops with 2 threads And 1/8th of the user cpu Interface to select between the 2 clients * More comments and backtrack test * http_test was missing from bazel ran gazelle * Added benchmark and fix folding bugs Folding is 4x faster than built in, 1/3rd of allocs Search is 10x faster, no alloc instead of a lot ``` BenchmarkASCIIFoldNormalToLower-8 3000000 416 ns/op 96 B/op 3 allocs/op BenchmarkASCIIFoldCustomToLowerMap-8 5000000 371 ns/op 96 B/op 3 allocs/op BenchmarkASCIIToUpper-8 20000000 104 ns/op 32 B/op 1 allocs/op BenchmarkFoldFind0-8 200000 7621 ns/op 2112 B/op 3 allocs/op BenchmarkFoldFind-8 2000000 669 ns/op 0 B/op 0 allocs/op ``` * chunked decoding unit test for parsing, reads the first chunk * fasthttp client wrapper (-http 3) - added some doc in WORKSPACE about wtool - initial support for fasthttp * Make linter happy though I might drop fasthttp as it’s not faster (but it’s more complete) and adds a lot of dependencies * Split connect out * Fixing merge error * adding fortio Logger * Fixing bazel build And misc comment/leftover from #460 etc * Shorter logger names * Switched to new logging * Eradicate Verbosity leftovers * Added -logprefix and -logcaller; Refactor start on http.go also fixed a couple of calls missing % codes * Minor edits, saving to GitHub wip * Woot http1.1 works now Still unwieldy but working ! * Some profiler optimizations * More profiler opt * Make lint happy by making checking connection closed header a flag * Dropping fasthttp dependency * Moved 3 changes to #495 * Minor: use version in echoers, use 3 digits * Code review updates (thx doug!) + serious chunked bug fix Wasn’t working with last chunk (0\r\n\r\n) being in a separate frame * Code review comment thx doug! Former-commit-id: e09dcaf

* Gateway hosts are required (istio#470) * Clarify gateway selector scope (istio#474) * Clarify gateway selector scope * fix indent * regen * k8s -> Kubernetes * Fix generated output

* Shrink and better document the debug image before: 722MB after: 236MB (Not counting envoy itself) Fixes istio#423 Needs istio/old_pilot_repo#1202 to fully work ```release-note Reduce proxy debug image by 2/3rd (486Mb) yet as functional ``` * Alignement fix

* Add remote profile * Update assets

Signed-off-by: Jack Tysoe <jack@tys.one> Signed-off-by: Pavel Nikolov <pavelnikolov@users.noreply.github.com> Co-authored-by: Pavel Nikolov <pavelnikolov@users.noreply.github.com>

ldemailly added 8 commits July 8, 2017 19:03

fortio update

d1b2134

- build and use fortio in perf/standalone scripts - (win) fix for having more connections than requested

Updating sample to test using echosrv (slightly lower latency)

6707ad8

Also changing threshold for sleep time histogram to 5%

Merge branch 'master' into ldemailly-bench6

993245e

Adding -gomaxprocs flag

e3a49a7

One client per goroutine

784f9bc

Now getting exactly the expected number of sockets But performance is still significantly lower than wrk

Adding profiler option

88205e2

Merge branch 'master' into ldemailly-bench6

3052cdd

Homegrown http client

a3e06d9

On my Mac: Old net/http : 19.7k qps with 2 threads (-http -1) New -http 1 : 30.5k ops with 2 threads And 1/8th of the user cpu Interface to select between the 2 clients Wip because it doesn’t work for chunked yet only for fixed Content-Length (like echosrv)

googlebot added the cla: yes label Jul 16, 2017

ldemailly mentioned this pull request Jul 16, 2017

fortio update #460

Merged

ldemailly added 2 commits July 16, 2017 01:22

More comments and backtrack test

f5308aa

http_test was missing from bazel

5372781

ran gazelle

ldemailly changed the title ~~Homegrown http client~~ fortio: optimized http client Jul 16, 2017

Merge branch 'ldemailly-bench6' into ldemailly-bench6-custhttp

3a429c7

ldemailly mentioned this pull request Jul 16, 2017

prow issues istio/test-infra#277

Closed

4 tasks

douglas-reid reviewed Jul 16, 2017

View reviewed changes

ldemailly and others added 2 commits July 16, 2017 10:53

Merge branch 'master' into ldemailly-bench6

da5c613

Merge branch 'ldemailly-bench6' into ldemailly-bench6-custhttp

0be9579

istio deleted a comment from istio-testing Jul 16, 2017

ldemailly added 2 commits July 16, 2017 15:54

Wip chunked decoding

01925bc

unit test for parsing, reads the first chunk

istio deleted a comment from istio-testing Jul 17, 2017

Partial fix for linter #471

d83b07d

The granularity seems not great no ?

istio deleted a comment from istio-testing Jul 17, 2017

fasthttp client wrapper (-http 3)

36aa6f2

- added some doc in WORKSPACE about wool - initial support for fasthttp

Minor: use version in echoers, use 3 digits

695746f

istio deleted a comment from istio-testing Jul 25, 2017

ldemailly mentioned this pull request Jul 25, 2017

fortio improvements #453

Closed

19 tasks

douglas-reid reviewed Jul 25, 2017

View reviewed changes

Code review updates (thx doug!) + serious chunked bug fix

a7ceb8a

Wasn’t working with last chunk (0\r\n\r\n) being in a separate frame

douglas-reid reviewed Jul 26, 2017

View reviewed changes

Merge branch 'master' into ldemailly-bench6-custhttp

ae7a5a3

Code review comment

1dd679a

thx doug!

istio deleted a comment from istio-testing Jul 26, 2017

ldemailly commented Jul 26, 2017

View reviewed changes

douglas-reid approved these changes Jul 27, 2017

View reviewed changes

ldemailly merged commit e09dcaf into master Jul 27, 2017

ldemailly deleted the ldemailly-bench6-custhttp branch August 7, 2017 18:15

guptasu pushed a commit to guptasu/istio that referenced this pull request Jun 11, 2018

Gateway hosts are required (istio#470)

813bdc7

guptasu pushed a commit to guptasu/istio that referenced this pull request Jun 11, 2018

Gateway hosts are required (istio#470)

4eac2c3

howardjohn pushed a commit to howardjohn/istio that referenced this pull request Jan 12, 2020

Add remote profile (istio#470)

67406f3

* Add remote profile * Update assets


		const toLowerMask byte = ('a' - 'A')

		// ASCIIFold returns a byte array equal to the input string but in lowercase.

fortio: optimized http client, simplified logging #470

fortio: optimized http client, simplified logging #470

Conversation

ldemailly commented Jul 16, 2017 • edited Loading

ldemailly commented Jul 16, 2017 • edited Loading

douglas-reid Jul 16, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ldemailly Jul 26, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ldemailly commented Jul 26, 2017

Choose a reason for hiding this comment

ldemailly Jul 26, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ayj commented Jul 26, 2017

ldemailly commented Jul 26, 2017 • edited Loading

ayj commented Jul 26, 2017 • edited Loading

Choose a reason for hiding this comment

douglas-reid left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ldemailly commented Jul 27, 2017

ldemailly commented Jul 16, 2017 •

edited

Loading

ldemailly commented Jul 16, 2017 •

edited

Loading

douglas-reid Jul 16, 2017 •

edited

Loading

ldemailly Jul 26, 2017 •

edited

Loading

ldemailly Jul 26, 2017 •

edited

Loading

ldemailly commented Jul 26, 2017 •

edited

Loading

ayj commented Jul 26, 2017 •

edited

Loading