It seems like the commit you're pointing to was a race bugfix, so it's not the type of performance regression that can be easily fixed or reverted. Do you have any ideas on how to get the numbers back to 1.10 levels?

@mvdan, I'm not familiar with net/http at all, so I can't really propose any solution or evaluate submitted one. This is why I've added this to description:

The commit message lacks benchmarks info, so I think it's worthwhile to report regression anyway and decide whether it can be addressed or not (we can't just remove locking if it's needed there, but there can be other options).

I've reported it for completeness as it showed pretty high delta.
It's better to know the impact than not.
Usually, it's better when CLs that affects performance in this way includes benchstat info, but this one didn't, hence here is the missing link.

@mvdan it's the defer, some goto hackary should make it pretty close to the old numbers.

If that's acceptable, I can push a PR sometime this week (or even better if you wanna do ;)).

Change https://golang.org/cl/113996 mentions this issue: net/http: avoid defer unlock in ServeMux.shouldRedirect

Change https://golang.org/cl/116378 mentions this issue: net/http: remove an allocation in ServeMux

@odeke-em, want to post an update on this now that 116378 is submitted? Or bump to Go 1.12 or Unplanned?

@bradfitz thanks for the ping.

So after CL 116378, we still have that regression from the newly introduced defer. I think we meant 116378 for #25482 instead :)

CL 113996 removes the defer that is the subject of this issue but it doesn't seem like you like its approach

@odeke-em - Just wanted to check on this. I just re-ran the benchmarks from the current tip 510eea2 and 1.10.2

name        old time/op    new time/op    delta
ServeMux-4    85.5µs ± 1%    78.4µs ± 1%   -8.26%  (p=0.008 n=5+5)

name        old alloc/op   new alloc/op   delta
ServeMux-4    19.9kB ± 0%    17.3kB ± 0%  -13.18%  (p=0.008 n=5+5)

name        old allocs/op  new allocs/op  delta
ServeMux-4       540 ± 0%       360 ± 0%  -33.33%  (p=0.008 n=5+5)

It seems we have a measurable improvement. Is there still anything else you would like to do on this ?

Thank you for the reminder @agnivade!

@carl-mastrangelo's nice CL https://go-review.googlesource.com/c/144937 aka commit 1645dfa massively levelled the ground so we are good now 1645dfa -- I had an almost similar change with longest prefix matching by sorting by length but didn't get the wins in his CL, cool stuff @carl-mastrangelo!

With that I am going to close this issue, @bradfitz or anyone else please feel free to reopen or bump to Unplanned.

@odeke-em I appreciate the call out, but I don't believe my change fixed this. I sent out https://go-review.googlesource.com/c/go/+/149377 which brings the latency down to the pre-lock levels.

I appreciate the call out, but I don't believe my change fixed this. I sent out https://go-review.googlesource.com/c/go/+/149377 which brings the latency down to the pre-lock levels.

@carl-mastrangelo, thanks for the humility! For clarity, I should have spelled out that this issue was combated by a collective of fixes:

and others that I might not have seen, which have brought the levels down to as @agnivade reported in #25383 (comment)

The issue originally was a regression in performance, which is fixed (even improved) now. Although, changes were made in other areas, but it eventually improved performance. I think this issue can be closed, because the regression does not exist any more.

Otherwise, we should re-title it to "net/http: remove mutex locks in ServeMux" because that is what this should be about.

I replied before I noticed that @bradfitz reopened it. @carl-mastrangelo was just stating that he didn't think his CL fixed this issue but I explained why I mentioned his CL and how it contributed to bringing the regressions down in #25383 (comment).

@bradfitz could you please reconsider closing this issue as it was about a regression that has been erased as mentioned in #25383 (comment) but also just by @agnivade in #25383 (comment)?

Sure. Carl, file a new bug if you want to track lock removal.