twilio-csharp icon indicating copy to clipboard operation
twilio-csharp copied to clipboard

feat: Use `XmlWriter` to serialize TwiML instead of using `XDocument`

Open Swimburger opened this issue 2 years ago • 4 comments

All third-party contributors acknowledge that any contributions they provide will be made under the same open-source license that the open-source project is provided under.

Currently the TwiML tree is constructed as a tree of objects which are then converted into a tree of objects from the System.Xml.Linq, to then be serialized to a string using XDocument.Save. This creates three copies of the same data in memory, one as TwiML objects, one as System.Xml.Linq objects, and one as a string.

To reduce the amount of memory used and increase performance, this PR uses the XmlWriter APIs instead of System.Xml.Linq APIs.

The TwiML tree is constructed just the same, but that tree is serialized without creating another copy of the tree, and can be written directly to Streams, TextWriters, and XmlWriters. If the Stream is a file stream, a network stream, etc., the entire XML string wouldn't be loaded into memory as a result. If you do use a TextWriter or the ToString method, the string would be loaded in memory but this PR won't create another copy of the tree as System.Xml.Linq objects.

The resulting XML uses self-closing tags at the moment, which would reduce the size of the string and make HTTP responses containing TwiML smaller, but this can be reverted if necessary for backwards compatibility reasons.

Checklist

  • [x] I acknowledge that all my contributions will be made under the project's license
  • [x] I have made a material change to the repo (functionality, testing, spelling, grammar)
  • [x] I have read the Contribution Guidelines and my PR follows them
  • [x] I have titled the PR appropriately
  • [x] I have updated my branch with the main branch
  • [x] I have added tests that prove my fix is effective or that my feature works
  • [x] I have added the necessary documentation about the functionality in the appropriate .md file
  • [x] I have added inline documentation to the code I modified

Swimburger avatar Feb 02 '23 17:02 Swimburger

C'mon Twilio folks, this code contribution has been ready to merge on the internal repo and public repo for a long time. The longer you let these things rot, the more conflicts will arise.

By not merging this, you're choosing to keep a slower and less memory-efficient version around for no reason. 🔥 🌳

Swimburger avatar Jan 17 '24 17:01 Swimburger