Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Use XmlWriter to serialize TwiML instead of using XDocument #669

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

Swimburger
Copy link
Contributor

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

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

@Swimburger
Copy link
Contributor Author

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. 🔥 🌳

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant