Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore(performance): Change *sync.RWMutex to sync.RWMutex #2351

Merged
merged 3 commits into from May 3, 2020

Conversation

appleboy
Copy link
Member

@appleboy appleboy commented May 3, 2020

fix #2350

See the report: #2350

Change *sync.RWMutex to sync.RWMutex. See the commet

step and result

Testing in my Macbook Pro

$ go get github.com/gin-gonic/gin@d5a42fc8ac93cebe42cde9c9b91a138358e4f99a
go: github.com/gin-gonic/gin d5a42fc8ac93cebe42cde9c9b91a138358e4f99a => v1.6.3-0.20200503042414-d5a42fc8ac93
go: downloading github.com/gin-gonic/gin v1.6.3-0.20200503042414-d5a42fc8ac93
$ go get github.com/julienschmidt/httprouter@master
go: github.com/julienschmidt/httprouter master => v1.3.1-0.20200114094804-8c9f31f047a3
go: downloading github.com/julienschmidt/httprouter v1.3.1-0.20200114094804-8c9f31f047a3

Benchmark Result:

BenchmarkEcho_Param              	12894638	        88.9 ns/op	       0 B/op	       0 allocs/op
BenchmarkGin_Param               	16120810	        75.8 ns/op	       0 B/op	       0 allocs/op
BenchmarkHttpRouter_Param        	20860491	        59.0 ns/op	       0 B/op	       0 allocs/op
BenchmarkEcho_Param5             	 5107472	       236 ns/op	       0 B/op	       0 allocs/op
BenchmarkGin_Param5              	 8665498	       139 ns/op	       0 B/op	       0 allocs/op
BenchmarkHttpRouter_Param5       	10716376	       111 ns/op	       0 B/op	       0 allocs/op
BenchmarkEcho_Param20            	 1649486	       736 ns/op	       0 B/op	       0 allocs/op
BenchmarkGin_Param20             	 3693909	       317 ns/op	       0 B/op	       0 allocs/op
BenchmarkHttpRouter_Param20      	 4302121	       291 ns/op	       0 B/op	       0 allocs/op
BenchmarkEcho_ParamWrite         	 6067071	       206 ns/op	       8 B/op	       1 allocs/op
BenchmarkGin_ParamWrite          	 7927640	       153 ns/op	       0 B/op	       0 allocs/op
BenchmarkHttpRouter_ParamWrite   	12336866	        97.2 ns/op	       0 B/op	       0 allocs/op
BenchmarkEcho_GithubStatic       	 9660526	       123 ns/op	       0 B/op	       0 allocs/op
BenchmarkGin_GithubStatic        	12108596	        99.3 ns/op	       0 B/op	       0 allocs/op
BenchmarkHttpRouter_GithubStatic 	22733364	        53.9 ns/op	       0 B/op	       0 allocs/op
BenchmarkEcho_GithubParam        	 4760176	       241 ns/op	       0 B/op	       0 allocs/op
BenchmarkGin_GithubParam         	 6903195	       175 ns/op	       0 B/op	       0 allocs/op
BenchmarkHttpRouter_GithubParam  	 8757140	       139 ns/op	       0 B/op	       0 allocs/op
BenchmarkEcho_GithubAll          	   25303	     47044 ns/op	       0 B/op	       0 allocs/op
BenchmarkGin_GithubAll           	   35906	     32017 ns/op	       0 B/op	       0 allocs/op
BenchmarkHttpRouter_GithubAll    	   48505	     24723 ns/op	       0 B/op	       0 allocs/op
BenchmarkEcho_GPlusStatic        	13986368	        88.1 ns/op	       0 B/op	       0 allocs/op
BenchmarkGin_GPlusStatic         	15715994	        76.2 ns/op	       0 B/op	       0 allocs/op
BenchmarkHttpRouter_GPlusStatic  	40313703	        30.6 ns/op	       0 B/op	       0 allocs/op
BenchmarkEcho_GPlusParam         	 8838810	       132 ns/op	       0 B/op	       0 allocs/op
BenchmarkGin_GPlusParam          	11063152	       110 ns/op	       0 B/op	       0 allocs/op
BenchmarkHttpRouter_GPlusParam   	14874406	        83.3 ns/op	       0 B/op	       0 allocs/op
BenchmarkEcho_GPlus2Params       	 5957268	       197 ns/op	       0 B/op	       0 allocs/op
BenchmarkGin_GPlus2Params        	 8538530	       138 ns/op	       0 B/op	       0 allocs/op
BenchmarkHttpRouter_GPlus2Params 	11009853	       109 ns/op	       0 B/op	       0 allocs/op
BenchmarkEcho_GPlusAll           	  610393	      2072 ns/op	       0 B/op	       0 allocs/op
BenchmarkGin_GPlusAll            	  845540	      1452 ns/op	       0 B/op	       0 allocs/op
BenchmarkHttpRouter_GPlusAll     	 1000000	      1022 ns/op	       0 B/op	       0 allocs/op
BenchmarkEcho_ParseStatic        	13372183	        89.4 ns/op	       0 B/op	       0 allocs/op
BenchmarkGin_ParseStatic         	15553862	        76.2 ns/op	       0 B/op	       0 allocs/op
BenchmarkHttpRouter_ParseStatic  	36294546	        32.0 ns/op	       0 B/op	       0 allocs/op
BenchmarkEcho_ParseParam         	10718779	       108 ns/op	       0 B/op	       0 allocs/op
BenchmarkGin_ParseParam          	14966005	        82.9 ns/op	       0 B/op	       0 allocs/op
BenchmarkHttpRouter_ParseParam   	18341719	        65.6 ns/op	       0 B/op	       0 allocs/op
BenchmarkEcho_Parse2Params       	 8146166	       149 ns/op	       0 B/op	       0 allocs/op
BenchmarkGin_Parse2Params        	11563095	       105 ns/op	       0 B/op	       0 allocs/op
BenchmarkHttpRouter_Parse2Params 	14198692	        83.3 ns/op	       0 B/op	       0 allocs/op
BenchmarkEcho_ParseAll           	  343478	      3427 ns/op	       0 B/op	       0 allocs/op
BenchmarkGin_ParseAll            	  467335	      2817 ns/op	       0 B/op	       0 allocs/op
BenchmarkHttpRouter_ParseAll     	  739195	      1768 ns/op	       0 B/op	       0 allocs/op
BenchmarkEcho_StaticAll          	   39229	     29002 ns/op	       0 B/op	       0 allocs/op
BenchmarkGin_StaticAll           	   50160	     25493 ns/op	       0 B/op	       0 allocs/op
BenchmarkHttpRouter_StaticAll    	   90253	     12674 ns/op	       0 B/op	       0 allocs/op
PASS
ok  	github.com/julienschmidt/go-http-routing-benchmark	67.399s

Or you can see the benchmark result from Travis: https://travis-ci.org/github/gin-gonic/go-http-routing-benchmark/jobs/682559844

@appleboy appleboy requested a review from thinkerou May 3, 2020
@codecov
Copy link

@codecov codecov bot commented May 3, 2020

Codecov Report

Merging #2351 into master will increase coverage by 0.08%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2351      +/-   ##
==========================================
+ Coverage   98.39%   98.48%   +0.08%     
==========================================
  Files          41       41              
  Lines        2304     2303       -1     
==========================================
+ Hits         2267     2268       +1     
+ Misses         21       20       -1     
+ Partials       16       15       -1     
Impacted Files Coverage Δ
context.go 98.08% <100.00%> (+0.42%) ⬆️
gin.go 99.13% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4427ca4...5b57025. Read the comment docs.

@thinkerou thinkerou added this to the 1.7 milestone May 3, 2020
@appleboy appleboy closed this May 3, 2020
@appleboy appleboy reopened this May 3, 2020
@appleboy appleboy changed the title chore(performance): remove Mutex lock in context key chore(performance): Enable/Disable Mutex lock in context key May 3, 2020
appleboy added 2 commits May 3, 2020
Signed-off-by: Bo-Yi Wu <appleboy.tw@gmail.com>
Signed-off-by: Bo-Yi Wu <appleboy.tw@gmail.com>
@appleboy appleboy changed the title chore(performance): Enable/Disable Mutex lock in context key chore(performance): Change *sync.RWMutex to sync.RWMutex May 3, 2020
@appleboy
Copy link
Member Author

@appleboy appleboy commented May 3, 2020

@thinkerou Please review again. I will bump to v1.6.3 after merge this PR.

@appleboy appleboy modified the milestones: 1.7, 1.6.3 May 3, 2020
@appleboy
Copy link
Member Author

@appleboy appleboy commented May 3, 2020

See the report from Travis: https://travis-ci.org/github/gin-gonic/go-http-routing-benchmark/jobs/682559844

$ go test -bench="Gin|Echo|HttpRouter"
#GithubAPI Routes: 203
   Echo: 100088 Bytes
   Gin: 58512 Bytes
   HttpRouter: 37144 Bytes
#GPlusAPI Routes: 13
   Echo: 9688 Bytes
   Gin: 4384 Bytes
   HttpRouter: 2808 Bytes
#ParseAPI Routes: 26
   Echo: 11872 Bytes
   Gin: 7776 Bytes
   HttpRouter: 5072 Bytes
#Static Routes: 157
   Echo: 80328 Bytes
   Gin: 34936 Bytes
   HttpRouter: 21712 Bytes
goos: linux
goarch: amd64
pkg: github.com/julienschmidt/go-http-routing-benchmark
BenchmarkEcho_Param              	16403503	        73.0 ns/op	       0 B/op	       0 allocs/op
BenchmarkGin_Param               	19200168	        63.0 ns/op	       0 B/op	       0 allocs/op
BenchmarkHttpRouter_Param        	24812588	        48.4 ns/op	       0 B/op	       0 allocs/op
BenchmarkEcho_Param5             	 6419178	       195 ns/op	       0 B/op	       0 allocs/op
BenchmarkGin_Param5              	11282056	       107 ns/op	       0 B/op	       0 allocs/op
BenchmarkHttpRouter_Param5       	12574981	        95.3 ns/op	       0 B/op	       0 allocs/op
BenchmarkEcho_Param20            	 2083293	       576 ns/op	       0 B/op	       0 allocs/op
BenchmarkGin_Param20             	 4224432	       269 ns/op	       0 B/op	       0 allocs/op
BenchmarkHttpRouter_Param20      	 4793692	       250 ns/op	       0 B/op	       0 allocs/op
BenchmarkEcho_ParamWrite         	 7633502	       171 ns/op	       8 B/op	       1 allocs/op
BenchmarkGin_ParamWrite          	 9792526	       124 ns/op	       0 B/op	       0 allocs/op
BenchmarkHttpRouter_ParamWrite   	14408402	        79.7 ns/op	       0 B/op	       0 allocs/op
BenchmarkEcho_GithubStatic       	11518838	       104 ns/op	       0 B/op	       0 allocs/op
BenchmarkGin_GithubStatic        	15070081	        80.0 ns/op	       0 B/op	       0 allocs/op
BenchmarkHttpRouter_GithubStatic 	27487160	        43.7 ns/op	       0 B/op	       0 allocs/op
BenchmarkEcho_GithubParam        	 5907873	       198 ns/op	       0 B/op	       0 allocs/op
BenchmarkGin_GithubParam         	 9415957	       128 ns/op	       0 B/op	       0 allocs/op
BenchmarkHttpRouter_GithubParam  	10633294	       115 ns/op	       0 B/op	       0 allocs/op
BenchmarkEcho_GithubAll          	   29670	     40039 ns/op	       0 B/op	       0 allocs/op
BenchmarkGin_GithubAll           	   44275	     27052 ns/op	       0 B/op	       0 allocs/op
BenchmarkHttpRouter_GithubAll    	   60016	     20138 ns/op	       0 B/op	       0 allocs/op
BenchmarkEcho_GPlusStatic        	17001352	        71.2 ns/op	       0 B/op	       0 allocs/op
BenchmarkGin_GPlusStatic         	18895076	        63.5 ns/op	       0 B/op	       0 allocs/op
BenchmarkHttpRouter_GPlusStatic  	48431677	        24.6 ns/op	       0 B/op	       0 allocs/op
BenchmarkEcho_GPlusParam         	11254729	       106 ns/op	       0 B/op	       0 allocs/op
BenchmarkGin_GPlusParam          	14198464	        84.6 ns/op	       0 B/op	       0 allocs/op
BenchmarkHttpRouter_GPlusParam   	15721663	        76.5 ns/op	       0 B/op	       0 allocs/op
BenchmarkEcho_GPlus2Params       	 7311615	       164 ns/op	       0 B/op	       0 allocs/op
BenchmarkGin_GPlus2Params        	11699445	       101 ns/op	       0 B/op	       0 allocs/op
BenchmarkHttpRouter_GPlus2Params 	11946534	       100 ns/op	       0 B/op	       0 allocs/op
BenchmarkEcho_GPlusAll           	  663595	      1740 ns/op	       0 B/op	       0 allocs/op
BenchmarkGin_GPlusAll            	 1000000	      1133 ns/op	       0 B/op	       0 allocs/op
BenchmarkHttpRouter_GPlusAll     	 1400403	       857 ns/op	       0 B/op	       0 allocs/op
BenchmarkEcho_ParseStatic        	16616858	        72.7 ns/op	       0 B/op	       0 allocs/op
BenchmarkGin_ParseStatic         	19508838	        61.6 ns/op	       0 B/op	       0 allocs/op
BenchmarkHttpRouter_ParseStatic  	46832966	        25.9 ns/op	       0 B/op	       0 allocs/op
BenchmarkEcho_ParseParam         	13397647	        89.1 ns/op	       0 B/op	       0 allocs/op
BenchmarkGin_ParseParam          	18163994	        66.1 ns/op	       0 B/op	       0 allocs/op
BenchmarkHttpRouter_ParseParam   	23268356	        51.1 ns/op	       0 B/op	       0 allocs/op
BenchmarkEcho_Parse2Params       	10108040	       119 ns/op	       0 B/op	       0 allocs/op
BenchmarkGin_Parse2Params        	15217050	        78.9 ns/op	       0 B/op	       0 allocs/op
BenchmarkHttpRouter_Parse2Params 	18317415	        66.1 ns/op	       0 B/op	       0 allocs/op
BenchmarkEcho_ParseAll           	  407364	      2879 ns/op	       0 B/op	       0 allocs/op
BenchmarkGin_ParseAll            	  602881	      1963 ns/op	       0 B/op	       0 allocs/op
BenchmarkHttpRouter_ParseAll     	  900546	      1322 ns/op	       0 B/op	       0 allocs/op
BenchmarkEcho_StaticAll          	   46124	     25657 ns/op	       0 B/op	       0 allocs/op
BenchmarkGin_StaticAll           	   60685	     19796 ns/op	       0 B/op	       0 allocs/op
BenchmarkHttpRouter_StaticAll    	  108180	     10973 ns/op	       0 B/op	       0 allocs/op
PASS
ok  	github.com/julienschmidt/go-http-routing-benchmark	65.607s

Copy link
Member

@thinkerou thinkerou left a comment

lgtm

@thinkerou
Copy link
Member

@thinkerou thinkerou commented May 3, 2020

@appleboy also update readme.me? thanks!

@appleboy
Copy link
Member Author

@appleboy appleboy commented May 3, 2020

@thinkerou What content in the readme I need to update?

@thinkerou
Copy link
Member

@thinkerou thinkerou commented May 3, 2020

@thinkerou What content in the readme I need to update?

@appleboy here https://github.com/gin-gonic/gin#benchmarks

@appleboy
Copy link
Member Author

@appleboy appleboy commented May 3, 2020

@thinkerou Yes. I know. I will take it after release v1.6.3 version. Thanks for your remind.

@appleboy appleboy merged commit 2c43278 into master May 3, 2020
4 checks passed
@appleboy appleboy deleted the performance branch May 3, 2020
maxatome added a commit to maxatome/gadgeto that referenced this issue May 4, 2020
See gin-gonic/gin#2351

Signed-off-by: Maxime Soulé <btik-git@scoubidou.com>
loopfz pushed a commit to loopfz/gadgeto that referenced this issue May 6, 2020
See gin-gonic/gin#2351

Signed-off-by: Maxime Soulé <btik-git@scoubidou.com>
@yongliu1992
Copy link

@yongliu1992 yongliu1992 commented Sep 6, 2020

@appleboy

Would you like to tell me Why *sync.RWMutex to sync.RWMutex can improvement performance ,I've read the code change, but I still don't know what the change solves to improve performance

@imxyb
Copy link
Contributor

@imxyb imxyb commented Sep 17, 2020

@appleboy i also want to know why improvement performance~

@gadilar
Copy link

@gadilar gadilar commented Sep 30, 2020

@appleboy

Would you like to tell me Why *sync.RWMutex to sync.RWMutex can improvement performance ,I've read the code change, but I still don't know what the change solves to improve performance

needn't share a lock for multiple contexts. *sync.RWMutex will share a lock after Context.Copy

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants