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

feat(router): add literal colon support (#1432) #2857

Merged
merged 1 commit into from
Jun 1, 2024

Conversation

wssccc
Copy link
Contributor

@wssccc wssccc commented Sep 6, 2021

Implement literal colon support using the approach described in #1432

@wssccc wssccc changed the title Add escaped colon support (#1432) Add literal colon support (#1432) Sep 6, 2021
@wssccc wssccc force-pushed the master branch 2 times, most recently from c1585c2 to 2e6a3ff Compare September 6, 2021 09:02
@codecov
Copy link

codecov bot commented Sep 6, 2021

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.06%. Comparing base (3dc1cd6) to head (451d0d0).
Report is 61 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2857      +/-   ##
==========================================
- Coverage   99.21%   99.06%   -0.16%     
==========================================
  Files          42       44       +2     
  Lines        3182     2772     -410     
==========================================
- Hits         3157     2746     -411     
+ Misses         17       15       -2     
- Partials        8       11       +3     
Flag Coverage Δ
?
-tags "sonic avx" 99.05% <100.00%> (?)
-tags go_json 99.05% <100.00%> (?)
-tags nomsgpack 99.04% <100.00%> (?)
go-1.18 ?
go-1.19 ?
go-1.20 ?
go-1.21 99.06% <100.00%> (-0.16%) ⬇️
go-1.22 99.06% <100.00%> (?)
macos-latest 99.06% <100.00%> (-0.16%) ⬇️
ubuntu-latest 99.06% <100.00%> (-0.16%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@wssccc wssccc force-pushed the master branch 4 times, most recently from 464c884 to ed0a30c Compare September 6, 2021 09:56
Copy link
Contributor

@daheige daheige left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still need to optimize and adjust, why define some routing rules like \?

gin.go Outdated Show resolved Hide resolved
@wssccc
Copy link
Contributor Author

wssccc commented Sep 6, 2021

Still need to optimize and adjust, why define some routing rules like ?

The feature included in this PR is not about defining rule like \, but using \ to escape a colon

Please refer to issue #1432 for more details

@wssccc wssccc force-pushed the master branch 2 times, most recently from 02a7101 to 6f0bdd0 Compare September 7, 2021 05:33
gin.go Outdated Show resolved Hide resolved
tree.go Outdated Show resolved Hide resolved
daheige
daheige previously approved these changes Apr 22, 2022
@wssccc
Copy link
Contributor Author

wssccc commented Apr 22, 2022

@cikay @daheige conflicts and issues mentioned above have been resolved

@mytram
Copy link

mytram commented Jul 13, 2022

Need this for my project which follows google's custom method conventions, i.e. :verb I am wondering if @cikay @daheige can review it and give it a tick. That will be awesome. Many thanks

@mytram
Copy link

mytram commented Jul 13, 2022

@daheige Thanks a lot 👍 🙏 !

@mytram
Copy link

mytram commented Jul 13, 2022

Hi @thinkerou wondering if you can have a look and approve the PR for the next release. Many thanks 🙏

@slnc
Copy link

slnc commented Nov 28, 2023

@cikay @daheige is anything blocking this PR? I'm asking in case the community can help. This PR would fix an issue my team is having.

Thank you

@appleboy appleboy added this to the v1.10 milestone Mar 11, 2024
@appleboy
Copy link
Member

I will take it asap.

@appleboy appleboy modified the milestones: v1.10, v1.11 Mar 21, 2024
tree.go Outdated Show resolved Hide resolved
@appleboy
Copy link
Member

@wssccc Can you also add http-router benchmark report since you update the router tree structure?

@appleboy appleboy changed the title Add literal colon support (#1432) feat(router): add literal colon support (#1432) May 23, 2024
@wssccc wssccc force-pushed the master branch 4 times, most recently from 12c06d9 to 43830be Compare May 23, 2024 03:56
@wssccc wssccc changed the base branch from master to benchmarks May 24, 2024 01:52
@wssccc wssccc changed the base branch from benchmarks to master May 24, 2024 01:53
@wssccc wssccc force-pushed the master branch 2 times, most recently from d84488e to e5e3228 Compare May 24, 2024 01:56
@appleboy
Copy link
Member

@wssccc I mean the different benchmarks between the master and this PR branch.

@appleboy
Copy link
Member

appleboy commented May 24, 2024

See the sample: #1826 (comment) using https://pkg.go.dev/golang.org/x/perf/cmd/benchstat tool to get benchmark result.

@wssccc
Copy link
Contributor Author

wssccc commented May 25, 2024

The pull request benchmark data:

@wssccc ➜ /workspaces/go-http-routing-benchmark (master) $ go test -bench=Gin
#GithubAPI Routes: 203
   Gin: 58792 Bytes

#GPlusAPI Routes: 13
   Gin: 4544 Bytes

#ParseAPI Routes: 26
   Gin: 7864 Bytes

#Static Routes: 157
   Gin: 34344 Bytes

goos: linux
goarch: amd64
pkg: github.com/julienschmidt/go-http-routing-benchmark
cpu: AMD EPYC 7763 64-Core Processor                
BenchmarkGin_Param              23936918                50.54 ns/op            0 B/op          0 allocs/op
BenchmarkGin_Param5             12072578               111.9 ns/op             0 B/op          0 allocs/op
BenchmarkGin_Param20             4816608               266.8 ns/op             0 B/op          0 allocs/op
BenchmarkGin_ParamWrite         19148685                64.69 ns/op            0 B/op          0 allocs/op
BenchmarkGin_GithubStatic       19514295                65.10 ns/op            0 B/op          0 allocs/op
BenchmarkGin_GithubParam         8956462               120.7 ns/op             0 B/op          0 allocs/op
BenchmarkGin_GithubAll             57808             19567 ns/op               0 B/op          0 allocs/op
BenchmarkGin_GPlusStatic        26049177                56.40 ns/op            0 B/op          0 allocs/op
BenchmarkGin_GPlusParam         15572720                84.69 ns/op            0 B/op          0 allocs/op
BenchmarkGin_GPlus2Params       11228659               100.1 ns/op             0 B/op          0 allocs/op
BenchmarkGin_GPlusAll            1303134               993.8 ns/op             0 B/op          0 allocs/op
BenchmarkGin_ParseStatic        26377340                48.55 ns/op            0 B/op          0 allocs/op
BenchmarkGin_ParseParam         18363444                60.13 ns/op            0 B/op          0 allocs/op
BenchmarkGin_Parse2Params       11881887                90.09 ns/op            0 B/op          0 allocs/op
BenchmarkGin_ParseAll             730262              1639 ns/op               0 B/op          0 allocs/op
BenchmarkGin_StaticAll             94722             14625 ns/op               0 B/op          0 allocs/op
PASS
ok      github.com/julienschmidt/go-http-routing-benchmark      27.432s

The master branch benchmark data:

@wssccc ➜ /workspaces/go-http-routing-benchmark (master) $ go test -bench=Gin
#GithubAPI Routes: 203
   Gin: 58792 Bytes

#GPlusAPI Routes: 13
   Gin: 4544 Bytes

#ParseAPI Routes: 26
   Gin: 7864 Bytes

#Static Routes: 157
   Gin: 34344 Bytes

goos: linux
goarch: amd64
pkg: github.com/julienschmidt/go-http-routing-benchmark
cpu: AMD EPYC 7763 64-Core Processor                
BenchmarkGin_Param              23772192                53.62 ns/op            0 B/op          0 allocs/op
BenchmarkGin_Param5             10336761               102.5 ns/op             0 B/op          0 allocs/op
BenchmarkGin_Param20             4763977               271.9 ns/op             0 B/op          0 allocs/op
BenchmarkGin_ParamWrite         15457245                73.56 ns/op            0 B/op          0 allocs/op
BenchmarkGin_GithubStatic       19980954                66.93 ns/op            0 B/op          0 allocs/op
BenchmarkGin_GithubParam        10229772               127.1 ns/op             0 B/op          0 allocs/op
BenchmarkGin_GithubAll             61861             20814 ns/op               0 B/op          0 allocs/op
BenchmarkGin_GPlusStatic        24932599                52.98 ns/op            0 B/op          0 allocs/op
BenchmarkGin_GPlusParam         16225578                80.40 ns/op            0 B/op          0 allocs/op
BenchmarkGin_GPlus2Params       12601467                99.77 ns/op            0 B/op          0 allocs/op
BenchmarkGin_GPlusAll            1204710              1090 ns/op               0 B/op          0 allocs/op
BenchmarkGin_ParseStatic        26572233                51.08 ns/op            0 B/op          0 allocs/op
BenchmarkGin_ParseParam         21089107                62.14 ns/op            0 B/op          0 allocs/op
BenchmarkGin_Parse2Params       16765136                73.76 ns/op            0 B/op          0 allocs/op
BenchmarkGin_ParseAll             737712              1910 ns/op               0 B/op          0 allocs/op
BenchmarkGin_StaticAll             85904             13133 ns/op               0 B/op          0 allocs/op
PASS
ok      github.com/julienschmidt/go-http-routing-benchmark      25.381s

@wssccc
Copy link
Contributor Author

wssccc commented May 25, 2024

@appleboy done

@appleboy appleboy merged commit 4621b7a into gin-gonic:master Jun 1, 2024
24 of 25 checks passed
@appleboy appleboy mentioned this pull request Jun 1, 2024
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.

7 participants