sway icon indicating copy to clipboard operation
sway copied to clipboard

Implement Serde for AstNode types

Open JoshuaBatty opened this issue 1 year ago • 2 comments

Description

This PR extends Serde support across all AstNode-related types, building upon the partial implementation from #4193. Key points:

  • Adds both Serialize and Deserialize trait implementations to all relevant types.
  • Prepares for upcoming language server features that will serialize/deserialize types between keystrokes.
  • Important: The type interface in the compiler is not yet stable. This implementation is intended for temporary serialization/deserialization work, not for persistent storage.

Checklist

  • [x] I have linked to any relevant issues.
  • [ ] I have commented my code, particularly in hard-to-understand areas.
  • [ ] I have updated the documentation where relevant (API docs, the reference, and the Sway book).
  • [ ] I have added tests that prove my fix is effective or that my feature works.
  • [ ] I have added (or requested a maintainer to add) the necessary Breaking* or New Feature labels where relevant.
  • [x] I have done my best to ensure that my PR adheres to the Fuel Labs Code Review Standards.
  • [x] I have requested a review from the relevant team or maintainers.

JoshuaBatty avatar Oct 04 '24 01:10 JoshuaBatty

  • Prepares for upcoming language server features that will serialize/deserialize types between keystrokes.

Could you maybe expand a bit on how this will work?

tritao avatar Oct 04 '24 13:10 tritao

  • Prepares for upcoming language server features that will serialize/deserialize types between keystrokes.

Could you maybe expand a bit on how this will work?

Hey @tritao sorry for the delay, just getting back to this work now. So the motivation for this was noticing really high RAM usage in the language server now. For example, just running the counter example we can see 800mb of heap memory is never deallocated after the first compilation pass.

On digging it seems this is due to the new module caching where we hold onto the types themselves, thus never allow Drop to be called.

#[derive(Clone, Debug)]
pub struct TypedModuleInfo {
    pub module: Arc<TyModule>,
    pub version: Option<u64>,
}

My motivation was to Serialize these types using postcard (same that is used in fuel-core) and then store the Serialize version in the module cache. We then need to Deserialize back into the original type on each keystroke which will incur some performance overhead but I haven't got any measurements yet, was going to tackle that in an up coming PR. WDYT, are you happy with the approach or do you think there is a better solution?

JoshuaBatty avatar Oct 29 '24 02:10 JoshuaBatty

CodSpeed Performance Report

Merging #6605 will not alter performance

Comparing josh/serde (712e825) with master (6d9065b)

Summary

✅ 22 untouched benchmarks

codspeed-hq[bot] avatar Nov 27 '24 03:11 codspeed-hq[bot]