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

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

Open
duckdoom4 opened this issue Aug 31, 2023 · 2 comments
Open

Comments

@duckdoom4
Copy link

duckdoom4 commented Aug 31, 2023

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.)

@duckdoom4 duckdoom4 changed the title Allow fbs fs_serializer to override global code gen settings Allow fbs fs_serializer attribute to override global code gen settings Aug 31, 2023
@jamescourtney
Copy link
Owner

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
Copy link
Author

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.

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

2 participants