Repository navigation
Faster in-memory writes and libxml issue detection - #112
Merged
Merged
Conversation
disallow_issues() wraps every Writer::write() and every DOM load. It went through detect_issues(), which builds a Psl Result, two closures and an IssueCollection on every call, even when libxml reported nothing. When the encoder writes one element per Writer, that overhead runs for every element. It now does the libxml bookkeeping itself and only builds the IssueCollection when libxml returned errors. The exceptions stay the same: a throwable from the callable is still wrapped with combineExceptionWithIssues() and keeps its message, code and previous, issues after a successful run still throw fromIssues(), and the libxml error state is restored on both paths. The new tests pin those parts down, including errors that are not exceptions.
The write loop called disallow_libxml_false_returns() for every value the builder generator yields, which is one extra function call per XMLWriter operation. The check is now inline and throws the same RuntimeException with the same message, which disallow_issues() wraps as before. The new test pins the wrapped exception and that writing stops at the first false.
configure() ran its stages through Psl\Iter\reduce with an extra closure per stage. Every Writer, Reader and Document goes through it, so the loop is now inline. It applies the stages in the same order and passes each result on to the next one, which the new test pins for the writer.
Writer::inMemory() went through memory_opener(), Writer::configure() and an empty configure() pipeline before it had an XMLWriter. It now opens XMLWriter::toMemory() directly, and fromUnsafeWriter() only builds the configure() pipeline when there are configurators to apply. With configurators the writer goes through them in order as before, and the new test pins that a writer returned by a configurator is the one that gets used.
Writer::write() called flush(false) after every write. For a file or a stream that pushes the content out, but for an XMLWriter::toMemory() buffer it does nothing anyone can observe: the buffer can only be read through flush() or outputMemory(), and both flush first. Writer::inMemory() now marks the writer it opened as in-memory and write() skips the flush for it. A configurator can hand back another XMLWriter or reopen the same one on a file, so only Writer::inMemory() without configurators gets the mark. The new tests pin that file and stream writers are still flushed after every write, also when they come out of an inMemory() configurator. Flipping the mark to false only adds a flush that nobody can observe, so I added that TrueValue mutant to the infection ignore list.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
php-soap/encoding writes every element through its own
Writer::inMemory(), so the fixed cost of one small write in this library adds up to a large share of an encode. Most of that cost sits in the bookkeeping around XMLWriter. This PR takes out the overhead on that path, one commit per change:disallow_issues()does the libxml error bookkeeping itself and only builds anIssueCollectionwhen libxml reported errors, so the happy path no longer allocates a PslResult, two closures and an empty collection. This also speeds up every DOM load.Writer::write()checks for afalsewrite inline instead of callingdisallow_libxml_false_returns()per yielded value.configure()folds its stages with a plain loop instead ofPsl\Iter\reduceand a closure per stage.Writer::inMemory()opensXMLWriter::toMemory()directly, andfromUnsafeWriter()skips theconfigure()pipeline when there are no configurators.write()skipsflush(false)for a writer opened byWriter::inMemory()without configurators. That buffer can only be read throughflush()oroutputMemory(), and both flush first. With configurators the writer is flushed as before, because a configurator can swap or reopen it.Compatibility
There is no public API or behaviour change. I compared a reflection dump of every class, method, function, parameter, return type and docblock type under
src/between 4.x and this branch, and it is identical. The exceptions on the error paths keep their type, message, code and previous exception, the libxml error state is restored on both paths, configurators run in the same order, and file and stream writers are still flushed after every write. I added tests that pin those parts where nothing did yet. I added one entry to the infection ignore list: thetruein the in-memory mark can only be mutated into an extra flush that nobody can observe.Speed
Measured on PHP 8.4 with php-soap/encoding as the consumer against this branch and 4.13.0, 3 interleaved rounds, median of the round medians. The 3MB workloads encode and decode a 3MB stdClass payload, the phpbench rows are the Encode, EncodeSparse and Decode subjects (x1 and x500), and the micro rows are hrtime loops.
Writer::inMemory()->write(element(...))->map(memory_output())disallow_issues()on the happy path