Skip to content

Conversation

@songjiayang
Copy link
Contributor

@songjiayang songjiayang commented Oct 12, 2022

Signed-off-by: songjiayang [email protected]

What this PR does:

Which issue(s) this PR fixes:
Fixes #4904

Checklist

  • Tests updated
  • Documentation added
  • CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX]

@songjiayang
Copy link
Contributor Author

I did a benchmark test, with cache has about 45%. less time cost.

benchmark code:

# add mux to mockTenantLimits
type mockTenantLimits struct {
	mux    sync.RWMutex
	limits map[string]*Limits
}

func (l *mockTenantLimits) ByUserID(userID string) *Limits {
	l.mux.RLock()
	defer l.mux.RUnlock()

	return l.limits[userID]
}

#  add benchmark method
func BenchmarkValidateLabelsNoCache(b *testing.B) {
	runBenchmarkValidateLabels(b, false)
}

func BenchmarkValidateLabelsWithCache(b *testing.B) {
	runBenchmarkValidateLabels(b, true)
}

func runBenchmarkValidateLabels(b *testing.B, cacheable bool) {
	defaultLimits := Limits{}
	flagext.DefaultValues(&defaultLimits)
	limits, _ := NewOverrides(defaultLimits, newMockTenantLimits(
		map[string]*Limits{
			"demo": &Limits{},
		},
	))
	if cacheable {
		limits, _ = NewOverrides(*(limits.GetOverridesForUser("demo")), nil)
	}

	ls := []cortexpb.LabelAdapter{
		{Name: model.MetricNameLabel, Value: "name"},
		{Name: "a", Value: "a"},
		{Name: "b", Value: "b"},
		{Name: "c", Value: "c"},
		{Name: "d", Value: "d"},
	}

	var wg sync.WaitGroup
	for i := 0; i < 5; i++ {
		wg.Add(1)
		go func() {
			for n := 0; n < b.N; n++ {
				ValidateLabels(limits, "demo", ls, false)
			}
			wg.Done()
		}()
	}
	wg.Wait()
}

The benchmark result is:

$ go test -run BenchmarkValidateLabels -bench=.  -benchtime=5s
goos: darwin
goarch: amd64
pkg: github.com/cortexproject/cortex/pkg/util/validation
cpu: Intel(R) Core(TM) i5-7360U CPU @ 2.30GHz
BenchmarkValidateLabelsNoCache-4     	 6538873	       899.9 ns/op
BenchmarkValidateLabelsWithCache-4   	 9504530	       622.5 ns/op

@pull-request-size pull-request-size bot added size/L and removed size/M labels Oct 13, 2022
@songjiayang songjiayang requested review from alanprot and removed request for alvinlin123 October 13, 2022 10:15
@alanprot
Copy link
Member

Thanks for working on this!

Can you just address the 2 nits and i can merge afterwards?

@songjiayang
Copy link
Contributor Author

code updated, review again please @alanprot

@alvinlin123 alvinlin123 merged commit fdf1563 into cortexproject:master Oct 17, 2022
@alvinlin123
Copy link
Contributor

Thanks for your contribution @songjiayang !

@songjiayang songjiayang deleted the cache-limits branch November 15, 2022 13:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

reduce the lock use times in ValidateLabels method

3 participants