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

Remove/reduce format! use in Interface::introspect_to_writer impl in interface #828

Open
zeenix opened this issue Jun 3, 2024 · 3 comments
Labels
enhancement New feature or request zbus Issues/PRs related to zbus crate

Comments

@zeenix
Copy link
Contributor

zeenix commented Jun 3, 2024

This is adding to the binary size (#304).

The problem is number of format! calls and format macro generating a lot of code. Possible solutions:

  • Convert the macros to create string literals instead. The issue is that we need some parameters that we don't have access to in the proc macro: indentation level and Type::signature() so it will not be as easy as simply moving the format! calls to the macro side.
  • We could look into using ufmt crate but it seems to be not well-maintained and hasn't seen a release for 2 years. Perhaps some other crate?
@zeenix zeenix added enhancement New feature or request zbus Issues/PRs related to zbus crate labels Jun 3, 2024
@zeenix zeenix changed the title Remove/reduce std::fmt::format use in Interface::introspect_to_writer impl in interface macro Remove/reduce format! use in Interface::introspect_to_writer impl in interface` Jun 3, 2024
@zeenix zeenix changed the title Remove/reduce format! use in Interface::introspect_to_writer impl in interface` Remove/reduce format! use in Interface::introspect_to_writer impl in interface Jun 13, 2024
@HookedBehemoth
Copy link

Could you elaborate on the issue with doing this at compile time some more? If it's all done then, indentation can be inferred there. And in my eyes, the signatures could also be obtained unless they are inferred.

@zeenix
Copy link
Contributor Author

zeenix commented Jul 17, 2024

Could you elaborate on the issue with doing this at compile time some more? If it's all done then, indentation can be inferred there.

You're right that the level argument of Interface::introspect_to_writer can be changed to a const generic and we can do so w/o bumping the major version since Interface is declared to be unstable API.

However, the indent argument is bumped internally for each level here. Perhaps, the inner indentation level should be something the Interface implementation (i-e the macro) should handle and caller of introspect_to_writer should just pass the main indentation level. 🤔

And in my eyes, the signatures could also be obtained unless they are inferred.

I only wish it was that easy. 😢 Currently in rust you can't concatenate strings at compile time unless they're themselves literals (trait constants can't work). I do have a plan now to make most signature handling (especially during ser/de) constant though, through parsed signatures (#882). I'm planning on working on this during the weekend/next week.

@zeenix
Copy link
Contributor Author

zeenix commented Nov 11, 2024

So I went ahead and removed most format! calls but it doesn't save a hell lot in busd. The real issues are:

  • The template strings being part of the binary.
  • The number of calls, since we make separate calls to writeln! (or fmt::Write::push_str in my branch) for each argument of every method and signal.

I am thinking of allowing interfaces to provide file paths to load introspection from. The will give users a way to completely eliminate both of the above (well not all the code but it'll be considerably less code to load a string from a file).

In the meantime, there #1136 that already helps a bit.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request zbus Issues/PRs related to zbus crate
Projects
None yet
Development

No branches or pull requests

2 participants