Skip to content

[WIP] accessor optimization #1847

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

Closed
wants to merge 3 commits into from
Closed

[WIP] accessor optimization #1847

wants to merge 3 commits into from

Conversation

c9s
Copy link
Contributor

@c9s c9s commented Apr 1, 2016

Implement property getter optimization to make getter methods 2 times faster

Optimization Targets

Simple getters

class Foo {
    protected $value;
    public function getValue() {
        return $this->value;
    }
}

Const accessor

class Foo {
    public function getTemplate() {
        return "base.html";
    }
}

Simple setters

class Foo {
    protected $template;
    public function setTemplate($template) {
        $this->template = $template;
    }
}

Changes

  • Added accessor struct to op_array struct for saving accessor
    information.
  • Added accessor checking in pass_two() for getter functions.
  • Added quick property getter in INIT_METHOD_CALL op handler

Benchmark Result

Getter Method Only

https://gist.github.com/8cd230a5601cbe38439661adf3caca0d

Without getter optimization (3 runs):
151.0169506073ms

With getter optimization (3 runs)
39.201021194458ms

Affect

Other Microbench result: https://gist.github.com/c9s/0273ac21631562724cabf86c42e86e32

@c9s c9s force-pushed the c9s/getter-optimize branch from 1ff767d to b399f0a Compare April 1, 2016 16:17
@c9s c9s changed the title Implement property getter optimization to make getter methods 2 times faster [WIP] Implement property getter optimization to make getter methods 2 times faster Apr 1, 2016
@staabm
Copy link
Contributor

staabm commented Apr 1, 2016

Would be great to see numbers in a real app, e.g. Wordpress

struct {
zend_uchar type; // the accessor type (getter or setter)
uint32_t property_offset;
} accessor;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not great to extend all op_arrays with rarely used fields for getters only.

@c9s c9s force-pushed the c9s/getter-optimize branch 2 times, most recently from d235421 to 656ffb7 Compare April 2, 2016 15:43
@c9s
Copy link
Contributor Author

c9s commented Apr 2, 2016

I am not quiet sure if the code below makes sense, however it fixes the tests:

                        if (nextop->result_type == IS_VAR) {
                            ZVAL_DEREF(retval);
                            ZVAL_COPY(EX_VAR(nextop->result.var), retval);
                            assigned_result = 1;
                        }

@c9s
Copy link
Contributor Author

c9s commented Apr 2, 2016

I thought copying value should be done by the code below?

ZVAL_COPY_VALUE(EX_VAR(nextop->result.var), retval);
if (UNEXPECTED(Z_OPT_COPYABLE_P(EX_VAR(nextop->result.var)))) {
    zval_copy_ctor_func(EX_VAR(nextop->result.var));
}

@c9s
Copy link
Contributor Author

c9s commented Apr 2, 2016

Tests are passing now. I will clean up and refine them tomorrow.

@c9s c9s force-pushed the c9s/getter-optimize branch 4 times, most recently from e721893 to 0409f98 Compare April 3, 2016 08:08
@c9s c9s changed the title [WIP] Implement property getter optimization to make getter methods 2 times faster Implement property getter optimization to make getter methods 2 times faster Apr 3, 2016
@c9s c9s force-pushed the c9s/getter-optimize branch 2 times, most recently from 2b42c73 to f70b140 Compare April 5, 2016 01:24
- Added accessor struct to op_array struct for saving accessor
  information.
- Added accessor checking in pass_two() for getter functions.
- Added quick property getter in INIT_METHOD_CALL op handler
- Added strict condition when compiling getter accessors
@c9s c9s force-pushed the c9s/getter-optimize branch from f70b140 to eec81a8 Compare April 5, 2016 01:36
@c9s c9s force-pushed the c9s/getter-optimize branch from 928fb64 to 92885df Compare April 6, 2016 10:31
@c9s c9s changed the title Implement property getter optimization to make getter methods 2 times faster [WIP] Implement property getter optimization to make getter methods 2 times faster Apr 6, 2016
@c9s c9s changed the title [WIP] Implement property getter optimization to make getter methods 2 times faster [WIP] accessor optimization Apr 7, 2016
@c9s
Copy link
Contributor Author

c9s commented Apr 14, 2016

will send this PR later when func info is ready is the core.

@c9s c9s closed this Apr 14, 2016
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.

4 participants