7 ms·

HTTP throughput regression from Go 1.7.5 to 1.8

152 points by 01walid 10y ago

bsaul 10y ago
bradfitz : "That was one of the biggest architectural changes in the net/http.Server in quite some time. I never did any benchmarking (or optimizations) after that change. " Sorry, what ? It's not like the http server of the stdlib is here only for doing hello world code samples... You would imagine those benchmark to be part of some CI process along with unit tests.
- jobvandervoort 10y ago
  Talking about this commit: https://github.com/golang/go/commit/faf882d1d427e8c8a9a1be00d8ddcab81d1e848e https://github.com/golang/go/commit/faf882d1d427e8c8a9a1be00...
- lmb 10y ago
  Benchmarks in CI are hard, because you need them to run in the exact same environment to make any sort of conclusions. But CI's are often noisy, virtualised, dockerized, whateverized. There is not much benefit in that.
  - bsaul 10y ago
    That could be an idea for a service. Provide an instance type with stable execution environment for benchmarking. Just stable, and not necessarily performant.
    - bluejekyll 10y ago
      I think it's harder than that though. You're trying to predict real-world numbers, and a hobbled test environment could show benchmark hotspots which might never occur on a real system. It could still be a good, but very expensive, service, but its hard for me to imagine something that would be one-size-fits all service that would accurately predict real world experience.
      - bsaul 10y ago
        At least it could catch some easy regression between version after some heavy refactoring, like this particular one. Better than the complete random performance of current VMs on cloud. It wouldn't be much harder than just making sure you're the only VM running on the hardware (disk & cpu) . Much like a "reserved instance", with real guarantees against side effects.
        bluejekyll 10y ago
        I think it could catch this, but it could just as easily miss that there was a huge degridation on HDD vs SSD; high ram vs. low ram; fast vs slow CPU; GPU vs no GPU. Obviously you can build test suites for each of these scenarios, but I think it would be expensive to run all of them. That's all I'm really saying, it's by no means a bad idea, I think it's a great idea, just going to require some upfront thought of what type of environment the software is going to run in.
      - icebraining 10y ago
        Why would it be very expensive?
        bluejekyll 10y ago
        I'm assuming that you'd run many benchmarks, not just "Hello world". Each one will run for some number of cycles, to make sure that you have a good mean from each run. So it's expensive b/c it consumes CPU and time on shared systems, and those cost money... so I think a service like this could potentially cost significantly more to operate than say travis-ci.
  - poooogles 10y ago
    I encountered this a while ago, I've ended up bechmarking in relation to a past change and ensuring the percentage difference it's within a margin. Not ideal but it's better than pure X vs Y.
OhSoHumble 10y ago
"Too late for Go 1.8, but we can look into performance during Go 1.9." That probably shouldn't be the response for a major performance regression in a release candidate. Looks like I'm sticking to Go 1.7 for however long it'll take before 1.9 is released.
- 01walid 10y ago
  That's why I posted this, too many people would be affected by such regression. And would prefer sticking to Go 1.7.x
  - romanovcode 10y ago
    > too many people would be affected by such regression. Yes, too many people do stupid "hello world" tests indeed. Maybe this is a problem with running "hello world" tests and not that much of a real-world problem. Let's see.
  - andy_ppp 10y ago
    Nope, hardly anyone with real workloads will be affected.
- lmb 10y ago
  So all your application does is accept connections, send hello world, and close them again?
  - OhSoHumble 10y ago
    Absolutely! I provide a "hello world as a service" platform.
    - Cthulhu_ 10y ago
      How does it compare to https://github.com/salvatorecordiano/hello-world-as-a-service https://github.com/salvatorecordiano/hello-world-as-a-servic..., which is written in JS / Node? Does yours do more requests / second? Do you have an enterprise plan?
deleted 10y ago
[deleted]
jerf 10y ago
As I've mentioned before [1], as the number starts getting too large, "requests per second" isn't a useful way of measuring the performance of a webserver, you're really more interested in "seconds per request overhead". The former makes this sound horrible and leads to headlines that make it sound like the entire web stack has lost 20% of its performance, which is terrible. The latter shows that the "request overhead" has gone from ~100us per request to ~120us or so, which is a lot more informative and tends to lead to better understanding what the situation is. This is not meant as an attack or a defense of Go. The facts are what the facts are. The point here is to suggest that people use terminology that is more informative and easier to understand. There are people for whom 20us per request extra is a sufficiently nasty issue that they will not upgrade. There are also a lot of people who are literally multiple orders of magnitude away from that even remotely mattering because their requests tend to take 120ms anyhow. Using "seconds per request overhead" both makes it easier to understand both the real performance impact with real times, and makes it easier to understand that we're just talking about the base overhead per request rather than the speed of the entire request. It might also discourage some of our, ah, more junior developers from being too focused on this metric. Why would I want to use a webserver that can only do 100,000 requests per second when I can use this one over here that can do 1,000,000 requests per second? If you look at it from the point of view that we're speaking about the difference between 10 microseconds and 1 microsecond, it becomes easier to see that if my requests are going to take 10 milliseconds on average, this is not a relevant stat to be worried about when choosing my webserver, and I should examine just the other differences instead, which may be a great deal more relevant to my use cases. Edit: Literally while I was typing this up I see at least three comments already complaining about this regression. My question to you, my honest question to you (because some of you may well be able to answer "yes", especially with some of the tasks Go gets used for), is: Are you really going to have a problem with this? Does the rest of your request really run in microseconds? It's actually pretty challenging in the web world to run in microseconds. It can be done, but a lot of the basic things you want to do end up like "hit a database" generally end up involving milliseconds, i.e., "thousands of microseconds". [1]: https://news.ycombinator.com/item?id=11187264 https://news.ycombinator.com/item?id=11187264
- OhSoHumble 10y ago
arussellsaw 10y ago
worth mentioning that this is only a noticeable performance regression in situations where the majority of the request is spent in http processing, eg 'hello world' handlers. Here is an example of the performance improvements i've seen in a real world application, admittedly heavily GC bound, but still the performance improvements are considerable: https://twitter.com/arussellsaw/status/819904231759085571 https://twitter.com/arussellsaw/status/819904231759085571
- Cthulhu_ 10y ago
  This is the real benchmark - compare performance with real, working software instead of microbenchmarks that show a small regression (okay a fairly big one in terms of percentage) on a very specific and unrealistic use case. I'd take 20 us performance degradation in one specific slice of code over a 50% performance increase overall any day. edit: english in the previous sentence is bad. You know what I mean. Small regression is fine if the overall speed is much better.
reimertz 10y ago
Why would it be too late? Isn't this the whole reason for release candidates? To find final major issues before releasing the next major version? If not, could someone please educate me?
- barrkel 10y ago
  The closer you are to a release, the bigger the blocker needs to be. If there was incorrect behaviour in a mainline use case, that would be much more significant than a performance regression. A 20% performance regression in a minimal http server (i.e. one that doesn't have any business logic) does not sound like a big problem to me; that kind of overhead would normally be dwarfed by database calls, and a 20% increase in the overhead doesn't sound like it's a large increase in what I'd expect to already be a very small number.
  - reimertz 10y ago
    Thanks for the clarification. So a similar situation in node.js-land would be if require('http') would get a worst-case scenario of a 20% performance hit, right? If this is the case, even I, who only run single instances of node, would think it would be a fairly big impact that i'd try to fix if I was the maintainer and still had the possibility to fix it.
    - Vendan 10y ago
      The issue is that's it's 20% of the http library's time, not 20% of your application's time. Put a large app on it, and now the regression is 0.02%... Does it still make sense to push everything back for that 0.02%?
- ktta 10y ago
  Usually, the release candidate is modified only if significant bugs are detected. But this this branded more of an implementation pitfall so I doubt it'll be fixed. You're absolutely right about asking if it can be fixed now rather than later (I was very surprised they wanted to wait till 1.9!), and thanks for asking that on there. 1.8 would be known for this bug in case of static site hosting since there are more req/s for that use case, if this did make the official release. It should be noted that it was tested against a hello world benchmark and it won't matter in higher payload cases when the limiting factor isn't the extra routine but the payload itself by a long shot.
akerro 10y ago
Why it is too late? He doesn't want to give any justification. Isn't the point of RC and community supported development to catch such cases before stable is published? Just make another RC.
- dawkins 10y ago
  It's answered here: https://github.com/golang/go/issues/18964#issuecomment-278307245 https://github.com/golang/go/issues/18964#issuecomment-27830... Once a release candidate is issued, only documentation changes and changes to address critical bugs should be made. In general the bar for bug fixes at this point is even slightly higher than the bar for bug fixes in a minor release. We may prefer to issue a release with a known but very rare crash than to issue a release with a new but not production-tested fix.
siscia 10y ago
As jerf mention I don't believe that this particular regression is going to be significant for the almost totally of the use cases (and the very few that are going to be touch by it probably are savy enough to test their performance before to deploy in production). What I believe is more serious is that this wasn't catch during the development, it could definitely be a worth trade off however we should be aware of it...
Matthias247 10y ago
If I understand the possible culprit commit (https://github.com/golang/go/commit/faf882d1d427e8c8a9a1be00d8ddcab81d1e848e https://github.com/golang/go/commit/faf882d1d427e8c8a9a1be00...) correctly then real world applications could still be faster than with the older versions on average. E.g. if a request handler would start a database request and forward it's CancellationToken (context.Done) to the database call both might be immediatly stopped with the new logic and the resources can be used for handling new requests. If in the old version the cancellation did not work properly the database request might have needed to run to completion before anything else could be done.
sddfd 10y ago
Conspiracy theory: They knew they'd take a 20 microseconds hit on every connection close, and (rightfully) did not care. So basically this is a communication issue with a community that does not understand what to make of its own benchmarks.
tmaly 10y ago
If you look at it, the change that was most attributed to the slow down, was committed on October 2016. Why could the people making an issue about the 0.5 us slow down per request not have tested or ran a benchmark sooner?
cameroncooper 10y ago
Surprised that nobody has mentioned the true hero of this story - git bisect - awesome tool, and perfect for pinpointing these sorts of regressions.
eternalban 10y ago
The std. dev. & max numbers caught my eyes: avg. std dev max Latency 195.30us 470.12us 16.30ms -- go tip Latency 192.49us 451.74us 15.14ms -- go 1.8rc3 Latency 210.16us 528.53us 14.78ms -- go 1.7.5 That is a seriously fat distribution. Has anyone ever benched for percentiles?
eternalban 10y ago
The std. dev. & max numbers caught my eyes: avg. std dev max Latency 195.30us 470.12us 16.30ms -- go tip Latency 192.49us 451.74us 15.14ms -- go 1.8rc3 Latency 210.16us 528.53us 14.78ms -- go 1.7.5 That is a seriously fat distribution. Has anyone ever benched for percentiles?

152 points by 01walid 10y ago

bsaul 10y ago

bradfitz : "That was one of the biggest architectural changes in the net/http.Server in quite some time. I never did any benchmarking (or optimizations) after that change. " Sorry, what ? It's not like the http server of the stdlib is here only for doing hello world code samples... You would imagine those benchmark to be part of some CI process along with unit tests.

jobvandervoort 10y ago
Talking about this commit: https://github.com/golang/go/commit/faf882d1d427e8c8a9a1be00d8ddcab81d1e848e https://github.com/golang/go/commit/faf882d1d427e8c8a9a1be00...
lmb 10y ago
Benchmarks in CI are hard, because you need them to run in the exact same environment to make any sort of conclusions. But CI's are often noisy, virtualised, dockerized, whateverized. There is not much benefit in that.
- bsaul 10y ago
  That could be an idea for a service. Provide an instance type with stable execution environment for benchmarking. Just stable, and not necessarily performant.
  - bluejekyll 10y ago
    I think it's harder than that though. You're trying to predict real-world numbers, and a hobbled test environment could show benchmark hotspots which might never occur on a real system. It could still be a good, but very expensive, service, but its hard for me to imagine something that would be one-size-fits all service that would accurately predict real world experience.
    - bsaul 10y ago
      At least it could catch some easy regression between version after some heavy refactoring, like this particular one. Better than the complete random performance of current VMs on cloud. It wouldn't be much harder than just making sure you're the only VM running on the hardware (disk & cpu) . Much like a "reserved instance", with real guarantees against side effects.
      - bluejekyll 10y ago
        I think it could catch this, but it could just as easily miss that there was a huge degridation on HDD vs SSD; high ram vs. low ram; fast vs slow CPU; GPU vs no GPU. Obviously you can build test suites for each of these scenarios, but I think it would be expensive to run all of them. That's all I'm really saying, it's by no means a bad idea, I think it's a great idea, just going to require some upfront thought of what type of environment the software is going to run in.
    - icebraining 10y ago
      Why would it be very expensive?
      - bluejekyll 10y ago
        I'm assuming that you'd run many benchmarks, not just "Hello world". Each one will run for some number of cycles, to make sure that you have a good mean from each run. So it's expensive b/c it consumes CPU and time on shared systems, and those cost money... so I think a service like this could potentially cost significantly more to operate than say travis-ci.
- poooogles 10y ago
  I encountered this a while ago, I've ended up bechmarking in relation to a past change and ensuring the percentage difference it's within a margin. Not ideal but it's better than pure X vs Y.

OhSoHumble 10y ago

"Too late for Go 1.8, but we can look into performance during Go 1.9." That probably shouldn't be the response for a major performance regression in a release candidate. Looks like I'm sticking to Go 1.7 for however long it'll take before 1.9 is released.

01walid 10y ago
That's why I posted this, too many people would be affected by such regression. And would prefer sticking to Go 1.7.x
- romanovcode 10y ago
  > too many people would be affected by such regression. Yes, too many people do stupid "hello world" tests indeed. Maybe this is a problem with running "hello world" tests and not that much of a real-world problem. Let's see.
- andy_ppp 10y ago
  Nope, hardly anyone with real workloads will be affected.
lmb 10y ago
So all your application does is accept connections, send hello world, and close them again?
- OhSoHumble 10y ago
  Absolutely! I provide a "hello world as a service" platform.
  - Cthulhu_ 10y ago
    How does it compare to https://github.com/salvatorecordiano/hello-world-as-a-service https://github.com/salvatorecordiano/hello-world-as-a-servic..., which is written in JS / Node? Does yours do more requests / second? Do you have an enterprise plan?

deleted 10y ago

[deleted]

jerf 10y ago

As I've mentioned before [1], as the number starts getting too large, "requests per second" isn't a useful way of measuring the performance of a webserver, you're really more interested in "seconds per request overhead". The former makes this sound horrible and leads to headlines that make it sound like the entire web stack has lost 20% of its performance, which is terrible. The latter shows that the "request overhead" has gone from ~100us per request to ~120us or so, which is a lot more informative and tends to lead to better understanding what the situation is. This is not meant as an attack or a defense of Go. The facts are what the facts are. The point here is to suggest that people use terminology that is more informative and easier to understand. There are people for whom 20us per request extra is a sufficiently nasty issue that they will not upgrade. There are also a lot of people who are literally multiple orders of magnitude away from that even remotely mattering because their requests tend to take 120ms anyhow. Using "seconds per request overhead" both makes it easier to understand both the real performance impact with real times, and makes it easier to understand that we're just talking about the base overhead per request rather than the speed of the entire request. It might also discourage some of our, ah, more junior developers from being too focused on this metric. Why would I want to use a webserver that can only do 100,000 requests per second when I can use this one over here that can do 1,000,000 requests per second? If you look at it from the point of view that we're speaking about the difference between 10 microseconds and 1 microsecond, it becomes easier to see that if my requests are going to take 10 milliseconds on average, this is not a relevant stat to be worried about when choosing my webserver, and I should examine just the other differences instead, which may be a great deal more relevant to my use cases. Edit: Literally while I was typing this up I see at least three comments already complaining about this regression. My question to you, my honest question to you (because some of you may well be able to answer "yes", especially with some of the tasks Go gets used for), is: Are you really going to have a problem with this? Does the rest of your request really run in microseconds? It's actually pretty challenging in the web world to run in microseconds. It can be done, but a lot of the basic things you want to do end up like "hit a database" generally end up involving milliseconds, i.e., "thousands of microseconds". [1]: https://news.ycombinator.com/item?id=11187264 https://news.ycombinator.com/item?id=11187264

OhSoHumble 10y ago

arussellsaw 10y ago

worth mentioning that this is only a noticeable performance regression in situations where the majority of the request is spent in http processing, eg 'hello world' handlers. Here is an example of the performance improvements i've seen in a real world application, admittedly heavily GC bound, but still the performance improvements are considerable: https://twitter.com/arussellsaw/status/819904231759085571 https://twitter.com/arussellsaw/status/819904231759085571

Cthulhu_ 10y ago
This is the real benchmark - compare performance with real, working software instead of microbenchmarks that show a small regression (okay a fairly big one in terms of percentage) on a very specific and unrealistic use case. I'd take 20 us performance degradation in one specific slice of code over a 50% performance increase overall any day. edit: english in the previous sentence is bad. You know what I mean. Small regression is fine if the overall speed is much better.

reimertz 10y ago

Why would it be too late? Isn't this the whole reason for release candidates? To find final major issues before releasing the next major version? If not, could someone please educate me?

barrkel 10y ago
The closer you are to a release, the bigger the blocker needs to be. If there was incorrect behaviour in a mainline use case, that would be much more significant than a performance regression. A 20% performance regression in a minimal http server (i.e. one that doesn't have any business logic) does not sound like a big problem to me; that kind of overhead would normally be dwarfed by database calls, and a 20% increase in the overhead doesn't sound like it's a large increase in what I'd expect to already be a very small number.
- reimertz 10y ago
  Thanks for the clarification. So a similar situation in node.js-land would be if require('http') would get a worst-case scenario of a 20% performance hit, right? If this is the case, even I, who only run single instances of node, would think it would be a fairly big impact that i'd try to fix if I was the maintainer and still had the possibility to fix it.
  - Vendan 10y ago
    The issue is that's it's 20% of the http library's time, not 20% of your application's time. Put a large app on it, and now the regression is 0.02%... Does it still make sense to push everything back for that 0.02%?
ktta 10y ago
Usually, the release candidate is modified only if significant bugs are detected. But this this branded more of an implementation pitfall so I doubt it'll be fixed. You're absolutely right about asking if it can be fixed now rather than later (I was very surprised they wanted to wait till 1.9!), and thanks for asking that on there. 1.8 would be known for this bug in case of static site hosting since there are more req/s for that use case, if this did make the official release. It should be noted that it was tested against a hello world benchmark and it won't matter in higher payload cases when the limiting factor isn't the extra routine but the payload itself by a long shot.

akerro 10y ago

Why it is too late? He doesn't want to give any justification. Isn't the point of RC and community supported development to catch such cases before stable is published? Just make another RC.

dawkins 10y ago
It's answered here: https://github.com/golang/go/issues/18964#issuecomment-278307245 https://github.com/golang/go/issues/18964#issuecomment-27830... Once a release candidate is issued, only documentation changes and changes to address critical bugs should be made. In general the bar for bug fixes at this point is even slightly higher than the bar for bug fixes in a minor release. We may prefer to issue a release with a known but very rare crash than to issue a release with a new but not production-tested fix.

siscia 10y ago

As jerf mention I don't believe that this particular regression is going to be significant for the almost totally of the use cases (and the very few that are going to be touch by it probably are savy enough to test their performance before to deploy in production). What I believe is more serious is that this wasn't catch during the development, it could definitely be a worth trade off however we should be aware of it...

Matthias247 10y ago

If I understand the possible culprit commit (https://github.com/golang/go/commit/faf882d1d427e8c8a9a1be00d8ddcab81d1e848e https://github.com/golang/go/commit/faf882d1d427e8c8a9a1be00...) correctly then real world applications could still be faster than with the older versions on average. E.g. if a request handler would start a database request and forward it's CancellationToken (context.Done) to the database call both might be immediatly stopped with the new logic and the resources can be used for handling new requests. If in the old version the cancellation did not work properly the database request might have needed to run to completion before anything else could be done.

sddfd 10y ago

Conspiracy theory: They knew they'd take a 20 microseconds hit on every connection close, and (rightfully) did not care. So basically this is a communication issue with a community that does not understand what to make of its own benchmarks.

tmaly 10y ago

If you look at it, the change that was most attributed to the slow down, was committed on October 2016. Why could the people making an issue about the 0.5 us slow down per request not have tested or ran a benchmark sooner?

cameroncooper 10y ago

Surprised that nobody has mentioned the true hero of this story - git bisect - awesome tool, and perfect for pinpointing these sorts of regressions.

eternalban 10y ago

The std. dev. & max numbers caught my eyes: avg. std dev max Latency 195.30us 470.12us 16.30ms -- go tip Latency 192.49us 451.74us 15.14ms -- go 1.8rc3 Latency 210.16us 528.53us 14.78ms -- go 1.7.5 That is a seriously fat distribution. Has anyone ever benched for percentiles?

eternalban 10y ago