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

Support for pretty-print serialized output? #65

Open
berkowski opened this issue Sep 6, 2022 · 7 comments
Open

Support for pretty-print serialized output? #65

berkowski opened this issue Sep 6, 2022 · 7 comments

Comments

@berkowski
Copy link

Any interest? I added support in an fork fo serde-json-core for a personal project and would be happy to submit a PR for review.

@ryan-summers
Copy link
Member

Mind posting an example of the output? And where would the pretty-print go? In impl core::fmt::Display?

@berkowski
Copy link
Author

I followed serde_json's lead and abstracted out the formatting. Then added "to_slice_pretty", "to_vec_pretty" and "to_string_pretty" functions (as well as *_with_format versions if one wanted to define their own formatter) using heapless::Vec and heapless::String types instead of those in std/alloc. I didn't go as far as using it for core::fmt::Display.

It a bit intrusive of a change. Here's the current diff:
master...berkowski:serde-json-core:pretty-print

In my project firmware for an STM32L0 I was serializing to a byte buffer, then using DMA to write to a uart. In the end I scrapped pretty-printing for this application, all those spaces and newlines added up and really required lots of more space.

Output looks like:

{
  "Ok": {
    "Status": {
      "battery": "Off",
      "voltage": 0,
      "current": 45076,
      "pressure": null,
      "temperature": null,
      "rtc": {
        "source": "LSI",
        "time": null
      },
      "releases": "Safe"
    }
  }
}

@ryan-summers
Copy link
Member

Yeah, from an embedded, no_std perspective, the spaces and newlines are purely detrimental when you can just do formatting on the PC-side. That being said, pretty-printing is definitely still nice for potential debugging. Do you know if the pretty-print functionality impacts binary size at all when unused? I'd imagine it shouldn't, but it always depends.

@berkowski
Copy link
Author

berkowski commented Sep 6, 2022 via email

@ryan-summers
Copy link
Member

Hmm indeed, there's a fair amount of complexity added. Is it possible to:

  1. Move all the pretty-formatting into a single file?
  2. Enable or disable pretty-formatting via compile time flags?

I'm generally unsure how useful pretty-formatting would be in this repository though, given that the target audience is resource-constrained users.

@berkowski
Copy link
Author

berkowski commented Sep 8, 2022

I think it's a no-go as implemented in my fork. Some quick tests show some sizable increases in binary size for armv6m targets using the "compact" version of the formatter compared to the current available serializer in 0.4. So there's a penalty even if pretty printing isn't used.

I think it's still worth pursing though; it just can't come at any cost to those whom don't want to use it. Compile-time flags here probably the best way to go about it. Something like a "pretty-print" feature enabling a "to_slice_pretty" function.

For my use case the JSON output is returned to the user as the response to commands given on a serial port. I chose JSON for this since it's a reasonable blend between human and machine readability. Pretty-printing initially helped make some of the more complex responses more human-legible, which really helped during the debugging process.

Edit: And "worth pursing" here means I'll poke at alternative impl's and see if I can come up with something suitable for a PR :)

@berkowski
Copy link
Author

I took another stab at it:

master...berkowski:serde-json-core:pretty-print2

  • Added an optional "pretty-print" feature
  • Moved serialization macros into their own module for use with both serializer options
  • Modeled pretty-print serializer and tests after existing implementation

Since the new serializer is behind an optional feature there is no change for users that don't want it. Even with the feature enabled there's no change in code size if the pretty-print serializers (to_slice_pretty, to_string_pretty, to_vec_pretty) aren't used.

If it looks promising I'll open up a PR for any additional work needed.

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