reactphp / promise

Promises/A implementation for PHP.
https://reactphp.org/promise/
MIT License
2.38k stars 146 forks source link

Improve memory consumption by using static child canceller callback without binding to parent promise #117

Closed clue closed 6 years ago

clue commented 6 years ago

Calling cancel() on a child promise that follows its parent promise now no longer causes a cyclic garbage reference in any exception trace as discussed in #46. This builds on top of the implementation approach in #115 and #116 and the ideas discussed in #46.

The gist here is that $promise->then()->cancel() now no longer causes any unexpected cyclic garbage reference and consumers of this package do not need to take special care of this.

Similar patches have been introduced with #115 and #116 to implement a similar logic for the resolution and cancellation callbacks and this PR takes advantage of this for its internal implementation.

Invoking the benchmarking example from #113 shows no effect because it does not use child promises. After patching this to use $promise->then()->cancel() instead of $promise->cancel(), this shows a very significant performance and memory improvement! Initially this peaked somewhere around 15 MB on my system taking 24s. After applying this patch, this script reports a constant memory consumption of around 0.6 MB taking 5s.

This PR actually includes a test that shows how garbage memory references are no longer an issue in any supported PHP version and how cancelling a child promise no longer causes any such references on its own (this means that this requires no effort on the consumer side).