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

pre-alloc search post body for index_search #341

Merged
merged 3 commits into from Sep 5, 2022
Merged

Conversation

trim21
Copy link
Contributor

@trim21 trim21 commented Aug 17, 2022

Pull Request

map initialization without length map cause many allocs, adding a fixed known length to avoid.

this is a minor change I guess I don't need to create a issue for it?

What does this PR do?

Fixes #<issue_number>

PR checklist

Please check if your PR fulfills the following requirements:

  • Does this PR fix an existing issue?
  • Have you read the contributing guidelines?
  • Have you made sure that the title is accurate and descriptive of the changes?

Thank you so much for contributing to Meilisearch!

@alallema
Copy link
Contributor

Hi @trim21,
Thank you so much for contributing to Meilisearch ❤️
I'm very sorry I didn't take care of your PR instead. I'm not sure to understand well your modification, I'm not so comfortable with make(). Also, unfortunately I won't be here next week but I'll take care of your PR when I get back on the 5th.
Thanks again

@trim21
Copy link
Contributor Author

trim21 commented Aug 26, 2022

map initlized without hint, may cause go to to alloc may times when setting items.

currently map[K]V{} is just same with make(make[K]V) (or make(make[K]V, 0))

package main

import (
	"runtime"
	"testing"
)

func BenchmarkMapMake(b *testing.B) {
	for i := 0; i < b.N; i++ {
+		m := make(map[int]int, 14)
		for j := 0; j < 14; j++ {
			m[j] = j
		}
		runtime.KeepAlive(m)
	}
}

func BenchmarkMap(b *testing.B) {
	for i := 0; i < b.N; i++ {
+		var m = map[int]int{}
		for j := 0; j < 14; j++ {
			m[j] = j
		}
	}
}
cpu: AMD Ryzen 7 5800X 8-Core Processor
BenchmarkMapMake-16      4550072               268.6 ns/op           577 B/op          1 allocs/op
BenchmarkMap-16          1926235               627.5 ns/op           918 B/op          3 allocs/op
PASS

Copy link
Contributor

@alallema alallema left a comment

Choose a reason for hiding this comment

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

Thanks for this PR @ and for contributing to Meilisearch ❤️
LGTM! And thanks you for the explanation

@alallema
Copy link
Contributor

alallema commented Sep 5, 2022

bors merge

@bors
Copy link
Contributor

bors bot commented Sep 5, 2022

@bors bors bot merged commit 326994c into meilisearch:main Sep 5, 2022
@trim21 trim21 deleted the trying branch September 5, 2022 12:50
@alallema alallema added the enhancement New feature or request label Oct 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants