trait Serialize uses reference to self instead of move.

[Grinkers]

This is a breaking change to Serialize and edn-derive. See https://github.com/edn-rs/edn-derive/pull/37 for compatibility. Clean-history version of https://github.com/edn-rs/edn-rs/pull/137

Rationale: We want to be able to do things like

// assume some valid struct `val`
let s1 = val.serialize();
val.foobar = 42;
let s2 = val.serialize();

It is both annoying and potentially very expensive to move the struct, deallocate, and then create a whole new one each time you may want to serialize. If your Serializable data is inside of another struct, that requires a huge amount of work (making sure things can be Cloned, do deep copies, etc just to free it immediately after. In embedded this is probably critical, as you'll need the original, clone, and string data before starting to clean up, so max memory usage will high.

As far as performance goes https://github.com/edn-rs/edn-rs/pull/144/files#diff-684ad70dd994bef2eaab407a0147b7b4ec3922c586fa15f9fe015d848abfc963L53-R54

edn                  time:   [2.8078 µs 2.8699 µs 2.9567 µs]
                        change: [-46.116% -44.053% -41.986%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 15 outliers among 100 measurements (15.00%)
  6 (6.00%) high mild
  9 (9.00%) high severe

This shows how expensive move vs borrow is (will continue to scale up depending on size).

[evaporei]

I think the performance improves because move does a drop, and deallocating memory usually is expensive.

[Grinkers]

@naomijub @evaporei As you can see in the last commit, we have a circular dependency issue. I'm open to suggestions on how to get out of the deadlock with releases.

[Grinkers]

I think the performance improves because move does a drop, and deallocating memory usually is expensive.

In the example, it's not just dropping, but having to recreate it each loop too, essentially doing a clone() each time. it's about half of the cost in this benchmark.

[naomijub]

@Grinkers Those pending actions are not configured anywhere else, it might be from when the PR was created. Let me know when to merge and I can force merge it. Don't know how to clean it from the pipeline

I managed to fix them

[Grinkers]

I'm going to merge, with the TODO. It's needed because of the circular dependency (one of the two repos will need a temporary pointer. it makes sense to keep it here, as this is the one initiating the breaking change). After derive PRs, I'll clean it up as we progress.