google / flatbuffers

FlatBuffers: Memory Efficient Serialization Library
https://flatbuffers.dev/
Apache License 2.0
23.36k stars 3.25k forks source link

Additional accessors in the generated Java code #5103

Closed shevek closed 4 years ago

shevek commented 5 years ago

It would make the library much easier to use if:

Other stuff which would be "nice:

Use case:

aardappel commented 5 years ago

__init is not a function the user should be calling.. but yes, I don't see see a reason why it can't be moved to the base class.. care to make a PR?

I don't understand the use case for getByteBufferPosition.. can you give an example?

Struct has almost nothing in it.. I suppose we can make a base class for it, but it wouldn't help much, especially as this is not user facing code.

You cannot generically iterate a FlatBuffer without type information (not beyond generic fields in the root object anyway).

shevek commented 5 years ago

If we have length-prefixed bytebuffers, calling ByteBufferUtil.getWithoutSizePrefix() allocates a new ByteBuffer wrapper (yes, I know it's just a pointer, but it's still a 80-ish byte allocation) and we can't afford this. We need to allocate exactly one flatbuffer wrapper and make it point at an arbitrary size-prefixed ByteBuffer WITHOUT doing any allocation.

Right now, we have the following objects: ByteBuffer (data, points to entire region) ByteBuffer (pointer, points to sub-region of data containing our object, size-prefixed) FlatBufferObject (initialized to data in pointer).

We COULD use one extra statically allocated ByteBuffer to point to a subset of the pointer buffer, but right now, we'd rather call __init with the table offset because it seems neater and uses less memory total.

shevek commented 5 years ago

For getByteBufferPosition(), it's to do with implementing something like max() when the FlatBufferObject is a flyweight, as presumably intended. To clone a flyweight, we need to clone the internal state, and that means we need access to it.

Assume we have an underlying ByteBuffer (data), and some caller routine has a statically allocated FlatBufferObject. A callee subroutine wants to keep a pointer to FlatBufferObject, but it cannot do so because the caller is going to mutate that object after the return. So the callee needs to clone the internal state of the given FlatBufferObject into its OWN statically allocated FlatBufferObject, but it can't, because it can't get the offset-pointer.

Such a callee is, for instance a min() routine, which wants to keep the minimum of a set of objects it's being called with, but the argument is always a flyweight.

maxpaynebupt commented 5 years ago

It would make the library much easier to use if:

  • __init(int, ByteBuffer) were defined in Table and Struct, not the specific subclass. This would actually make the codebase smaller overall.
  • There was a getByteBufferPosition() { return bb_pos; } method - this would make it possible to clone an object-wrapper.

Other stuff which would be "nice:

  • A superinterface of Table and Struct for the purpose of common methods (mostly ByteBuffer stuff)

Use case:

  • Writing an iterator over a large ByteBuffer of an unknown flatbuf type. I can do it, but my iterator has to be parameterized with a ton of lambdas, and I need an extra weird hack to get bb_pos.

hi, could u please give some demo in your snmp4j project ? thanks.

aardappel commented 5 years ago

Rather than getByteBufferPosition it would probably better to have an explicit clone method?

I am not sure I understand your use of min & max in this context.

shevek commented 5 years ago

clone() would require an allocation. We already have the object wrapper allocated. Consider:

ByteBuffer data = ...
ByteBuffer slice = data.duplicate();
Foo foo = new Foo();
int offset = 0;
while (offset < data.size()) {
   int length = data.getInt();
   slice.offset(offset);
   slice.limit(offset + length());
   foo.__init(slice, 0);
   process(foo);

OK, so we can iterate a ByteBuffer and read Foo objects out of it without allocation. So far, so good. Now, let's have:

class MaxOfFoo {
    private final Foo max = new Foo();
    private final Comparator<T> comparator;
    void process(Foo value) {
        if (comparator.compare(value, max) > 0)
              max.__init(value.getByteBuffer(), value.getByteBufferPosition());  // <-------- THIS METHOD
    }
}

That method does not exist. We can't get the buffer out of an existing object without reflection. We don't know which buffer a given Foo came from.

shevek commented 5 years ago

Also: Defining __init in the super-interface would make this be MaxOf<T> not a special MaxOfFoo per type.

aardappel commented 5 years ago

Ok, I suppose it doesn't hurt to have a getByteBufferPosition for such uses. Care to make a PR?

stale[bot] commented 4 years ago

This issue has been automatically marked as stale because it has not had activity for 1 year. It will be automatically closed if no further activity occurs. To keep it open, simply post a new comment. Maintainers will re-open on new activity. Thank you for your contributions.

shevek commented 4 years ago

As far as I know, the issue is still extant.

github-actions[bot] commented 4 years ago

This issue is stale because it has been open 6 months with no activity. Please comment or this will be closed in 14 days.