Skip to content
This repository has been archived by the owner on Jan 12, 2024. It is now read-only.

Make DisplayableException more serialization-friendly #776

Merged
merged 2 commits into from
Mar 26, 2023

Conversation

minestarks
Copy link
Member

@minestarks minestarks commented Feb 28, 2023

Fixes https://dev.azure.com/ms-quantum/Quantum%20Program/_workitems/edit/44472

Serializing DisplayableException to JSON causes an OutOfMemoryException to be thrown.

JSON tracing shows hundreds of objects referenced through StackFrame attempted to be serialized:

Newtonsoft.Json Information: 0 : Finished serializing System.RuntimeType. Path 'StackTrace[0].Callable.__Body__.Method.Module.Assembly.DefinedTypes[263]'.
Newtonsoft.Json Information: 0 : Started serializing System.RuntimeType. Path 'StackTrace[0].Callable.__Body__.Method.Module.Assembly.DefinedTypes[263]'.
Newtonsoft.Json Information: 0 : Finished serializing System.RuntimeType. Path 'StackTrace[0].Callable.__Body__.Method.Module.Assembly.DefinedTypes[264]'.
Newtonsoft.Json Information: 0 : Started serializing System.RuntimeType. Path 'StackTrace[0].Callable.__Body__.Method.Module.Assembly.DefinedTypes[264]'.

On principle I don't think an object designed to be serialized/displayed should have references to complicated types such as StackFrame. So I'm introducing a DisplayableStackFrame, a simple struct.

Additionally I'm cleaning up a few areas that I thought looked odd - where we were serializing the same string slightly differently, unnecessary null handling, etc.

Considered, and rejected:

  • Adding a custom JSON converter for DisplayableException: designing the object to be simple in the first place seems like a more sensible approach
  • Avoid serializing to JSON in the first place: This turned out to be way trickier than it sounds. Serializing results to JSON seems to be pretty baked into the IQ# engine, and the JSON results are used by the Python wrapper (namely in capture_diagnostics)

@minestarks minestarks merged commit 402aab6 into microsoft:main Mar 26, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants