jamescourtney / FlatSharp

Fast, idiomatic C# implementation of Flatbuffers
Apache License 2.0
510 stars 51 forks source link

Allow fbs fs_serializer attribute to override global code gen settings #402

Open duckdoom4 opened 1 year ago

duckdoom4 commented 1 year ago

Hi there, me again.

Turns out we do have one single table that used the Progressive serializer scheme, defined in the fbs using the attributes. table MyTable (fs_serializer:"Progressive")

Since we have GreedyMutable set in the project settings this no longer works.

<PropertyGroup>
  <FlatSharpDeserializers>GreedyMutable</FlatSharpDeserializers>
</PropertyGroup>

We were wondering if it would be possible to have the fbs override the global settings. We still don't want Progressive serializers to generate for any other files, but only for this single specific file.

If it's not easy to only look at the fbs attributes, maybe an added option like "AndIfSpecified" to enable that at a global level. (If omitted it'll still globally cause it to ignore the fbs attributes.)

jamescourtney commented 1 year ago

You can specify multiple serializers in the XML:

     <FlatSharpDeserializers>GreedyMutable;Progressive</FlatSharpDeserializers>

This will apply globally. You'll get (slightly) more code than you perhaps bargained for. You could put your progressive table in a separate .csproj, which might reduce the volume of code depending on how simple the one progressive table is. It's also possible it goes the other way and increases your net lines of code. Happy to explain why if you're curious.

FlatSharp could support the case you are describing, which would involve traversing the dependency graph to determine which types are dependents of the given root type, then overriding the generated serializers property for those types, etc. FlatSharp does this already for cycle detection to avoid emitting recursion depth checks for non-cyclical schemas. This is a medium sized chunk of work that I'm happy to add as a feature request, though I can't promise I'll get to it on any schedule.

Lines-of-code seems like a persistent issue for you, so let me set some context about why FlatSharp does what it does. Most people using FlatBuffers do so because they have performance needs that aren't met by other formats/libraries. It follows that FlatSharp generates code with the goal of generating C# that the JIT can translate into efficient assembly. Due to some decisions by the JIT, this sometimes requires generating more code than you would write if implementing things by hand with the goal of avoiding virtual method calls, generic method sharing, etc. The main changes to generated code size can be found in the release notes for 7.0.0 and 7.1.0.

HTH, James

duckdoom4 commented 1 year ago

Yeah the goal was to not add the additional code generation for those files that didn't need it and ideally we wouldn't have to create a seperate project for this specific file.

We have like 100 fbs files in one project so the small bits of extra code quickly add up to a lot of extra code. As we've seen before, that slowed down compilation times and introduced IDE lag so we'd like to avoid adding more then needed.

That said, since it's already much better than is was before the new changes were introduced it's probably okay to enable it globally for the project that needs it. So that's what we'll do for now.