go-ldap / ldap

Basic LDAP v3 functionality for the GO programming language.
Other
2.19k stars 352 forks source link

feat: Refactor `ParseDN` function to fix resource usage and invalid parsings (fixes #487) #497

Closed cpuschma closed 3 months ago

cpuschma commented 3 months ago

This PR reworks the ParseDN function, which has been very resource consuming for reading one byte at a time and couldn't handle certain characters. This implementation is based upon inteons PR for cert-manager, which wasn't fully compatible with the RFC 4514.

This PR also:

If this resolves the reported problems, we can revert #466.

cpuschma commented 3 months ago

Additionally, results of some benchmarking benchmark:

Summary

goos: linux
goarch: amd64
pkg: github.com/go-ldap/ldap
cpu: AMD Ryzen 5 5600X 6-Core Processor             
                │   old.txt    │               new.txt               │
                │    sec/op    │   sec/op     vs base                │
ParseSubject-12   2392.0n ± 1%   754.2n ± 1%  -68.47% (p=0.000 n=15)

                │   old.txt   │              new.txt               │
                │    B/op     │    B/op     vs base                │
ParseSubject-12   1865.0 ± 0%   408.0 ± 0%  -78.12% (p=0.000 n=15)

                │  old.txt   │              new.txt               │
                │ allocs/op  │ allocs/op   vs base                │
ParseSubject-12   58.00 ± 0%   10.00 ± 0%  -82.76% (p=0.000 n=15)

Old implementation:

BenchmarkParseSubject-12          673922          2258 ns/op        1865 B/op         58 allocs/op
BenchmarkParseSubject-12          519794          2403 ns/op        1865 B/op         58 allocs/op
BenchmarkParseSubject-12          458138          2411 ns/op        1865 B/op         58 allocs/op
BenchmarkParseSubject-12          426906          2440 ns/op        1865 B/op         58 allocs/op
BenchmarkParseSubject-12          502710          2369 ns/op        1865 B/op         58 allocs/op

New implementation::

BenchmarkParseSubject-12         1606474           773.1 ns/op       408 B/op         10 allocs/op
BenchmarkParseSubject-12         1554079           768.1 ns/op       408 B/op         10 allocs/op
BenchmarkParseSubject-12         1594885           760.0 ns/op       408 B/op         10 allocs/op
BenchmarkParseSubject-12         1564299           754.2 ns/op       408 B/op         10 allocs/op
BenchmarkParseSubject-12         1547341           755.9 ns/op       408 B/op         10 allocs/op