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

Optimize allocations with thisrc #13

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
Draft

Optimize allocations with thisrc #13

wants to merge 2 commits into from

Conversation

SOF3
Copy link
Member

@SOF3 SOF3 commented Aug 16, 2020

This pull request optimizes unique-reference calls to Vector3, which minimizes object allocations for long call chains like this:

$vector
  ->add(1, 2, 3)
  ->multiply(3)
  ->cross($otherVector)
  ->divide(4)

For the above code, the current implementation requires 4 allocations, while with this patch only one allocation is required.

Adaptability

This patch is not effective when the vector is referenced by any variables, including $this.
This means not even calling $this->xxx() from a subclass of Vector3 would utilize this optimization,
nor $foo->xxx() (even if $foo is no longer used).

This patch is not effective in some chain calls. For example, this does not optimize anything in $event->getPlayer()->add().

Drawbacks

This requires users to install ext-thisrc. A polyfill or cpp-based alternative may be used to support non-thisrc users.

This involves extra function call overhead in the thisrc() calls. ext-thisrc may be implemented at a lower level of PHP internals (rather than just as a PHP_FUNCTION) may be useful, but that would be much more challenging.

Tasks

  • Adopt thisrc in Vector3
  • Adopt thisrc in Vector2
  • Implement polyfill for users without ext-thisrc installed
  • Perform benchmarks to contrast the overhead of thisrc() against overhead of cloning. In particular, benchmark this in PocketMine, where not too many Vector3 calls are chained, to see if this makes anything worse.
  • Fix travis builds

I can't figure out how to clone an object in a PHP extension yet,
so this patch, which has more boilerplate, is used instead.
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