Skip to content

Commit

Permalink
Speed up NextLabel and PrevLabel (miekg#1039)
Browse files Browse the repository at this point in the history
* change NextLabel and PrevLabel to be faster

This reduces readability, but they are in the hot path of coredns.

* @redyeti pointed out, that my implementation disregarded triple backslashes

* add synthetic benchmark-tests for PrevLabel and NextLabel

* rename ii -> j

* invert compare

* PrevLabel: add empty string check + test case

* NextLabel: fix and add testcase for NextLabel("", offset>0)
  • Loading branch information
jpicht authored and aanm committed Jul 29, 2022
1 parent 00cecff commit d8d05e6
Show file tree
Hide file tree
Showing 2 changed files with 133 additions and 28 deletions.
60 changes: 42 additions & 18 deletions labels.go
Expand Up @@ -126,20 +126,23 @@ func Split(s string) []int {
// The bool end is true when the end of the string has been reached.
// Also see PrevLabel.
func NextLabel(s string, offset int) (i int, end bool) {
quote := false
if s == "" {
return 0, true
}
for i = offset; i < len(s)-1; i++ {
switch s[i] {
case '\\':
quote = !quote
default:
quote = false
case '.':
if quote {
quote = !quote
continue
}
return i + 1, false
if s[i] != '.' {
continue
}
j := i - 1
for j >= 0 && s[j] == '\\' {
j--
}

if (j-i)%2 == 0 {
continue
}

return i + 1, false
}
return i + 1, true
}
Expand All @@ -149,17 +152,38 @@ func NextLabel(s string, offset int) (i int, end bool) {
// The bool start is true when the start of the string has been overshot.
// Also see NextLabel.
func PrevLabel(s string, n int) (i int, start bool) {
if s == "" {
return 0, true
}
if n == 0 {
return len(s), false
}
lab := Split(s)
if lab == nil {
return 0, true

l := len(s) - 1
if s[l] == '.' {
l--
}
if n > len(lab) {
return 0, true

for ; l >= 0 && n > 0; l-- {
if s[l] != '.' {
continue
}
j := l - 1
for j >= 0 && s[j] == '\\' {
j--
}

if (j-l)%2 == 0 {
continue
}

n--
if n == 0 {
return l + 1, false
}
}
return lab[len(lab)-n], false

return 0, n > 1
}

// equal compares a and b while ignoring case. It returns true when equal otherwise false.
Expand Down
101 changes: 91 additions & 10 deletions labels_test.go
Expand Up @@ -40,16 +40,17 @@ func TestCompareDomainName(t *testing.T) {

func TestSplit(t *testing.T) {
splitter := map[string]int{
"www.miek.nl.": 3,
"www.miek.nl": 3,
"www..miek.nl": 4,
`www\.miek.nl.`: 2,
`www\\.miek.nl.`: 3,
".": 0,
"nl.": 1,
"nl": 1,
"com.": 1,
".com.": 2,
"www.miek.nl.": 3,
"www.miek.nl": 3,
"www..miek.nl": 4,
`www\.miek.nl.`: 2,
`www\\.miek.nl.`: 3,
`www\\\.miek.nl.`: 2,
".": 0,
"nl.": 1,
"nl": 1,
"com.": 1,
".com.": 2,
}
for s, i := range splitter {
if x := len(Split(s)); x != i {
Expand Down Expand Up @@ -79,12 +80,32 @@ func TestSplit2(t *testing.T) {
}
}

func TestNextLabel(t *testing.T) {
type next struct {
string
int
}
nexts := map[next]int{
{"", 1}: 0,
{"www.miek.nl.", 0}: 4,
{"www.miek.nl.", 4}: 9,
{"www.miek.nl.", 9}: 12,
}
for s, i := range nexts {
x, ok := NextLabel(s.string, s.int)
if i != x {
t.Errorf("label should be %d, got %d, %t: nexting %d, %s", i, x, ok, s.int, s.string)
}
}
}

func TestPrevLabel(t *testing.T) {
type prev struct {
string
int
}
prever := map[prev]int{
{"", 1}: 0,
{"www.miek.nl.", 0}: 12,
{"www.miek.nl.", 1}: 9,
{"www.miek.nl.", 2}: 4,
Expand Down Expand Up @@ -237,3 +258,63 @@ func BenchmarkIsSubDomain(b *testing.B) {
IsSubDomain("miek.nl.", "aa.example.com.")
}
}

func BenchmarkNextLabelSimple(b *testing.B) {
b.ReportAllocs()
for i := 0; i < b.N; i++ {
NextLabel("www.example.com", 0)
NextLabel("www.example.com", 5)
NextLabel("www.example.com", 12)
}
}

func BenchmarkPrevLabelSimple(b *testing.B) {
b.ReportAllocs()
for i := 0; i < b.N; i++ {
PrevLabel("www.example.com", 0)
PrevLabel("www.example.com", 5)
PrevLabel("www.example.com", 12)
}
}

func BenchmarkNextLabelComplex(b *testing.B) {
b.ReportAllocs()
for i := 0; i < b.N; i++ {
NextLabel(`www\.example.com`, 0)
NextLabel(`www\\.example.com`, 0)
NextLabel(`www\\\.example.com`, 0)
}
}

func BenchmarkPrevLabelComplex(b *testing.B) {
b.ReportAllocs()
for i := 0; i < b.N; i++ {
PrevLabel(`www\.example.com`, 10)
PrevLabel(`www\\.example.com`, 10)
PrevLabel(`www\\\.example.com`, 10)
}
}

func BenchmarkNextLabelMixed(b *testing.B) {
b.ReportAllocs()
for i := 0; i < b.N; i++ {
NextLabel("www.example.com", 0)
NextLabel(`www\.example.com`, 0)
NextLabel("www.example.com", 5)
NextLabel(`www\\.example.com`, 0)
NextLabel("www.example.com", 12)
NextLabel(`www\\\.example.com`, 0)
}
}

func BenchmarkPrevLabelMixed(b *testing.B) {
b.ReportAllocs()
for i := 0; i < b.N; i++ {
PrevLabel("www.example.com", 0)
PrevLabel(`www\.example.com`, 10)
PrevLabel("www.example.com", 5)
PrevLabel(`www\\.example.com`, 10)
PrevLabel("www.example.com", 12)
PrevLabel(`www\\\.example.com`, 10)
}
}

0 comments on commit d8d05e6

Please sign in to comment.