Skip to content

Reduce memory usage #402

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

Closed
wants to merge 1 commit into from
Closed

Conversation

SoftStoneDevelop
Copy link

Usage:

var producerConfig = new DeduplicatingProducerConfig(...)
{
    ConfirmationHandler = async confirmation =>
    {
        // call dispose on each confirmation.Messages or do nothing
    },
};

var producer = await DeduplicatingProducer.Create(producerConfig, ...);
// start producer...

// send new message
var memory = MemoryPool<byte>.Shared.Rent(10);
// fill memory...

await producer.Send(
    messageId,
    new Message(memory, 10));

@Gsantomaggio
Copy link
Member

Thank you for your PR. I will check it as soon as possible

@Gsantomaggio
Copy link
Member

@SoftStoneDevelop, may I ask for a report on the memory gain? We'd like to see some numbers. We will change the Message class, which is critical for the library and we have to be sure.
Thank you.

Copy link
Contributor

@lukebakken lukebakken left a comment

Choose a reason for hiding this comment

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

This is the sort of change that requires more planning. It makes sense to add *Memory-specific APIs to the project, but I'm not thrilled with how it becomes this library's responsibility for managing the lifetime of the externally-rented memory.

@SoftStoneDevelop - when you open a pull request, the best practice is to do so from a change-specific branch on your fork. Since you have opened a PR from main, I can't push changes to your fork.

I'm going to close this PR and start a discussion for version 2.0.

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.

3 participants