Skip to content

use pointers to optimize equalTopologies func #2014

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

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

isopov
Copy link
Contributor

@isopov isopov commented Apr 10, 2025

Optimize equalTopologies

Background & Motivation

In our idle service this method in about 2 hours generates about 1GB of allocations. I want to reduce CPU load on GC from this idle service.
image

Included benchmark on my machine produces the following results before

BenchmarkEqualTopologies
BenchmarkEqualTopologies-32    	   82350	     13399 ns/op	   90195 B/op	     206 allocs/op
PASS

and after

BenchmarkEqualTopologies
BenchmarkEqualTopologies-32    	  198764	      8329 ns/op	    6992 B/op	       6 allocs/op
PASS

this change.

servers := make([]description.Server, 100)
for i := 0; i < len(servers); i++ {
servers[i] = description.Server{
Addr: address.Address("127.0.0." + strconv.Itoa(i) + ":27017"),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

BTW, changing this string to

Addr:              address.Address("127.0.0." + strconv.Itoa(i)),

turns benchmark results into

BenchmarkEqualTopologies
BenchmarkEqualTopologies-32    	  133854	      8716 ns/op	   35793 B/op	     906 allocs/op
PASS

@isopov
Copy link
Contributor Author

isopov commented Apr 11, 2025

I've introduced NPD issue in my change, so I fixed it and added another test.

@isopov isopov requested a review from a team as a code owner April 14, 2025 10:31
@isopov isopov requested a review from qingyang-hu April 14, 2025 10:31
@codeowners-service-app codeowners-service-app bot requested a review from jtazin April 14, 2025 10:36
}

if len(topoServers) != len(otherServers) {
return false
}

for _, server := range topoServers {
otherServer := otherServers[server.Addr.String()]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've understood that we can reuse already computed canonical string for the address from the map key.

Benchmarking (different machine from previous results) produce slight improvement:

  • Before
BenchmarkEqualTopologies
BenchmarkEqualTopologies-10    	  281943	      4715 ns/op	    6992 B/op	       6 allocs/op
PASS
  • After
BenchmarkEqualTopologies
BenchmarkEqualTopologies-10    	  317918	      4228 ns/op	    6992 B/op	       6 allocs/op
PASS

And with non-canonical address (address.Address("127.0.0." + strconv.Itoa(i)))

  • Before
BenchmarkEqualTopologies
BenchmarkEqualTopologies-10    	   66252	     17721 ns/op	   35793 B/op	     906 allocs/op
PASS
  • After
BenchmarkEqualTopologies
BenchmarkEqualTopologies-10    	   91142	     12960 ns/op	   26193 B/op	     606 allocs/op
PASS

So we have pretty big additional improvement for free

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant