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

[Java] DTOs to support encoding into direct buffer via instance method. #1023

Open
the-thing opened this issue Nov 13, 2024 · 2 comments
Open

Comments

@the-thing
Copy link

Currently it is possible to encode DTO using the following methods:

  • extension.CarDto#encodeWith
  • extension.CarDto#encodeWithHeaderWith
  • extension.CarDto#bytes
  • extension.CarDto#bytesWithHeader

These methods are static so using them can cause a lot of duplication. Example.

class Sender1
{
    private final RingBuffer publisher;

    Sender1(final RingBuffer publisher)
    {
        this.publisher = publisher;
    }

    public boolean send(final CarDto car)
    {
        final int encodedLength = car.computeEncodedLength();
        final int index = publisher.tryClaim(1, encodedLength);

        if (index <= 0)
        {
            return false;
        }

        final AtomicBuffer buffer = publisher.buffer();

        try
        {
            CarDto.encodeWith(car, buffer, index);
        }
        catch (final Exception e)
        {
            publisher.abort(index);
            return false;
        }

        publisher.commit(index);

        return true;
    }

    // crate new method and repeat the same for another DTO
    public void send(final FooDto foo)
    {
    }
}

Adding computeEncodedLength was great since now it makes easier to estimate the object size to be used with io.aeron.Publication#tryClaim or org.agrona.concurrent.ringbuffer.RingBuffer#tryClaim.

Can we create a common interface such as e.g.:

public interface DtoEncoder
{

    int encode(MutableDirectBuffer buffer, int offset);

    int encodeWithHeader(MutableDirectBuffer buffer, int offset);

    // implementation already available	
    int computeEncodedLength();

    int computeEncodedLengthWithHeader();
}

that generates DTO code similar to:

public final class CarDto implements DtoEncoder
{
    @Override
    public int encode(final MutableDirectBuffer buffer, final int offset)
    {
        return encodeWith(this, buffer, offset);
    }

    @Override
    public int encodeWithHeader(final MutableDirectBuffer buffer, final int offset)
    {
        return encodeWithHeaderWith(this, buffer, offset);
    }

    @Override
    public int computeEncodedLengthWithHeader()
    {
        return MessageHeaderEncoder.ENCODED_LENGTH + computeEncodedLength();
    }
}

This would simplify encoding so all DTOs can be serialized into a direct buffer with a common interface.

class Sender2 {

    private static final int MSG_TYPE = 1;
    private final RingBuffer publisher;

    public Sender2(RingBuffer publisher) {
        this.publisher = publisher;
    }

    // can be used for all DTOs
    public boolean send(DtoEncoder encoder) {
        int encodedLength = encoder.computeEncodedLength();
        int index = publisher.tryClaim(MSG_TYPE, encodedLength);

        if (index <= 0) {
            return false;
        }

        AtomicBuffer buffer = publisher.buffer();

        try {
            encoder.encode(buffer, index);
        } catch (Exception e) {
            publisher.abort(index);
            return false;
        }

        publisher.commit(index);

        return true;
    }
}

I have a prototype for this change, but I this would require creating a common interface for DTOs in Agrona org.agrona.sbe. Let me know what are your thoughts.

@vyazelenko
Copy link
Contributor

@ZachBray Any thoughts?

@ZachBray
Copy link
Contributor

ZachBray commented Dec 2, 2024

It sounds like a reasonable change to me.

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

No branches or pull requests

3 participants