feat: Support cleaning up spare examples correctly in read_span_flatbuffer() #4684

lokitoth · 2024-02-14T19:13:21Z

Consumers of VW as a library can provide their own event pools, etc. Previous parsers were always able to predict when an even would be needed ahead of time, so would only allocate when necessary. This was done by relying on a single incoming event preallocation to let the external host deallocate in the case of nothing to be parsed.

This does not work for the FB parser due to how it handles re-entrancy, and we do not want to spend the time re-architecting it to avoid this. The fix, in this case, is to expand the API to include a callback to return spare events back to the host's event pool.

Consumers of VW as a library can provide their own event pools, etc. Previous parsers were always able to predict when an even would be needed ahead of time, so would only allocate when necessary. This was done by relying on a single incoming event preallocation to let the external host deallocate in the case of nothing to be parsed. This does not work for the FB parser due to how it handles re-entrancy, and we do not want to spend the time re-architecting it to avoid this. The fix, in this case, is to expand the API to include a callback to return spare events back to the host's event pool.

The C++ compiler on our image of Ubuntu 2004 cannot properly pick up Offset<> => Offset<void>. Make this explicit.

vowpalwabbit/fb_parser/src/parse_example_flatbuffer.cc

vowpalwabbit/fb_parser/include/vw/fb_parser/parse_example_flatbuffer.h

vowpalwabbit/fb_parser/src/parse_example_flatbuffer.cc

vowpalwabbit/fb_parser/src/parse_label.cc

When re-using the flatbuffer parser across multiple invocations, the parser state could become invalid (retain references to deleted objects) * Add more tests for bad inputs * Add comments about what is going on in read_span_flatbuffer * Add some comments about the re-entrant logic in parse_examples * Fix a place where the parser was returning the semantically incorrect error code

The code to generate bad namespaces in examples was interacting poorly with auto.

…wrong error code

vowpalwabbit/fb_parser/src/parse_example_flatbuffer.cc

rajan-chari

Looks good. Thanks Jacob.

* also add assertion around incoming examples.size() == 1 (or implictly 0)

lokitoth and others added 5 commits February 14, 2024 14:12

fix: Clean up spare examples properly when an exception is thrown

dba1c45

fix: api_status.h macros colide between VW and RL

5df10a5

feat: Add additional error reporting to read_span_flatbuffer

b7c7748

fix: Build break on Ubuntu 2004

1895dcc

The C++ compiler on our image of Ubuntu 2004 cannot properly pick up Offset<> => Offset<void>. Make this explicit.