rust-embedded-community / serde-json-core

`serde-json` for `no_std` programs
Apache License 2.0
161 stars 59 forks source link

Support for pretty-print serialized output? #65

Open berkowski opened 2 years ago

berkowski commented 2 years ago

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 commented 2 years ago

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

berkowski commented 2 years ago

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: https://github.com/rust-embedded-community/serde-json-core/compare/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 commented 2 years ago

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 commented 2 years ago

I can build up some comparisons over the next few days to explore things like that. Anything else you'd like to see besides binary size? Would size be a good enough proxy to gauge complexity?

On Tue, Sep 6, 2022, 12:56 Ryan Summers @.***> wrote:

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.

— Reply to this email directly, view it on GitHub https://github.com/rust-embedded-community/serde-json-core/issues/65#issuecomment-1238414856, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABBNRGGZBF5ONEMMBHTGIZ3V45ZUBANCNFSM6AAAAAAQF2DFRM . You are receiving this because you authored the thread.Message ID: @.***>

ryan-summers commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

I took another stab at it:

https://github.com/rust-embedded-community/serde-json-core/compare/master...berkowski:serde-json-core:pretty-print2

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.