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 progress callback without binding to promise #115

Closed clue closed 6 years ago

clue commented 6 years ago

This builds on top of the ideas discussed in #46. The third $progress callback passed to the resolver is deprecated as of #112 and as such should not be used for most applications.

This PR ensures that this callback never causes a cyclic garbage reference as discussed in #46. This builds on top of the work done in #113. If you do not access any arguments, this PR has no effect (also no performance or memory penalty).

If you do access any of the arguments, you no longer have to care about this method memory wise, as it will no longer cause a cyclic garbage reference. A similar patch will follow for the other two callbacks, but I figured it's worth discussing this concept on its own first.

Invoking the benchmarking example from #113 shows that this has no little effect on performance (1M invocations that do no use arguments show no performance improvements, 1M invocations that do use arguments show a ~4% performance degradation).

This PR actually includes a test that shows how garbage memory references can be avoided in PHP 7+ and how this method does no longer cause any such references on its own.