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

Do not call Debugf for non-debug levels in doDropRPC #580

Merged
merged 2 commits into from
Sep 25, 2024

Conversation

algorandskiy
Copy link
Contributor

@algorandskiy algorandskiy commented Sep 24, 2024

In high load scenarios when consumer is slow, doDropRPC is called often and makes extra unnecessary allocations invoking log.Debug.

Fixed by checking log level before running expensive formatting.

Before:

BenchmarkAllocDoDropRPC-10    	13684732	        76.28 ns/op	     144 B/op	       3 allocs/op

After:

BenchmarkAllocDoDropRPC-10    	28140273	        42.88 ns/op	     112 B/op	       1 allocs/op

@algorandskiy algorandskiy changed the title do not format expensive debug messages in non-debug levels in doDropRPC Do not format expensive debug messages in non-debug levels in doDropRPC Sep 24, 2024
Copy link
Collaborator

@vyzo vyzo left a comment

Choose a reason for hiding this comment

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

nice one.

@vyzo
Copy link
Collaborator

vyzo commented Sep 24, 2024

Can you go mod tidy please?

@algorandskiy
Copy link
Contributor Author

done.

There is also extra allocation in Message.GetFrom() on []byte to peer.ID conversion (runtime.slicebytetostring) but I'm not sure how to fix it properly. One approach could be caching []byte -> peer.ID on a first call and reuse on subsequent calls. Another one is to use []byte instead of peer.ID everywhere but it does not look right (although I'm not sure why peer.ID is a string and not []byte as it contains binary data).

@vyzo vyzo merged commit f71345c into libp2p:master Sep 25, 2024
9 checks passed
@vyzo
Copy link
Collaborator

vyzo commented Sep 25, 2024

peer.ID is a string so that you can easily compare and use as a key in maps, there is a lot of convenience to that.

@algorandskiy algorandskiy changed the title Do not format expensive debug messages in non-debug levels in doDropRPC Do not call Debugf for non-debug levels in doDropRPC Sep 26, 2024
@algorandskiy
Copy link
Contributor Author

I updated the PR title/description to properly denote the source of extra allocations: it is strings escaping b/c of interface{} in Debugf signature, and not the message formatting itself that happens later inside Debugf after checking the log level (I've confused it with fmt.Sprintf() in doDropRPC callers.

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.

2 participants