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

Add support for pooling in codecs, partition_table, view etc. #445

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

frairon
Copy link
Contributor

@frairon frairon commented Jan 1, 2024

Work in Progress

This is a draft for evaluating the efficiency and consequences of adding (memory) pooling to the goka API.

How it works

When processing messages with goka Processors, messages are decoded using a Codec in every callback call, as well as for every Join or Lookup. The same happens on every View.Get. When using more complex codecs like protobuf, those allocations add up and create pressure in the garbage collector. We would like to try reusing the messages. Protobuf supports Reseting the message upon unmarshalling, so they're a god fit for pooling.
Therefore we need to be able to tell the Codec when the message processing is done and the reference can be reused.

This PR adds a new decode-function called DecodeP which returns an io.Closer interface that must be called when the message can be reused.
Additionally, it also adds a GetP method to the storage.Storage interface to support pooling on disk-level, especially for Views.

Copy link

@paulhenke paulhenke left a comment

Choose a reason for hiding this comment

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

Therefore we need to be able to tell the Codec when the message processing is done and the reference can be reused.

So codecs become stateful? Or were there already before?

codec.go Outdated Show resolved Hide resolved
codec/closer.go Outdated Show resolved Hide resolved
codec/closer.go Outdated
Comment on lines 7 to 11
type nullCloser struct{}

func (n *nullCloser) Close() error { return nil }

var NoopCloser = new(nullCloser)

Choose a reason for hiding this comment

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

What is this for, if all the code calling DecodeP is doing a nil check on the io.Closer return.

codec/codec.go Outdated Show resolved Hide resolved
codec.go Show resolved Hide resolved
context.go Show resolved Hide resolved
context.go Show resolved Hide resolved
iterator.go Show resolved Hide resolved
iterator.go Show resolved Hide resolved
context.go Show resolved Hide resolved
@frairon
Copy link
Contributor Author

frairon commented Jan 7, 2024

Thanks @paulhenke for all your comments. As said in some comments, the idea of this PR is to draft how pooling could be supported, although goka does not actually use it, only the clients. The codecs would become stateful in some sense, that's true, but they haven't been before and don't have to be. But breaking the API for this is probably a bad idea, so we'll design it in a way that both ways are supported.
But again, we're just in testing/drafting phase now and want to see if implementing all that actually brings some performance gains. If not, the whole thing is dropped right away. So the styling improvements will follow later.

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.

None yet

2 participants