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

MM optimizations #834

Merged
merged 3 commits into from Jul 13, 2013
Merged

MM optimizations #834

merged 3 commits into from Jul 13, 2013

Conversation

ghost
Copy link

@ghost ghost commented Jul 12, 2013

  • grow should be 0 when MM is not used
  • do not use MM if no memory allocation takes place

@phalcon
Copy link
Collaborator

phalcon commented Jul 12, 2013

Well, this could lead to some errors, when a method fails it tries to restore the memory stack automatically so what could happens is that it eventually will restore the wrong memory frame (a one created before) or produce a segfault if none is available

@ghost
Copy link
Author

ghost commented Jul 12, 2013

OK, I see, let me update the code

@ghost
Copy link
Author

ghost commented Jul 12, 2013

OK, what do you think about this: all the cases above are for forwarding methods, e.g.,

PHP_METHOD(Phalcon_DI, offsetSet){

    zval *name, *definition;

    phalcon_fetch_params(0, 2, 0, &name, &definition);

    phalcon_call_method_p2_noret(this_ptr, "setshared", name, definition);
}

basically the method is equivalent to

public function offsetSet($name, $definition)
{
    $this->setShared($name, $definition);
}

and offers only yet another level of indirection.

The idea is to use PHP_MALIAS() for such methods:

PHP_ME(Phalcon_DI, setShared, arginfo_phalcon_di_setshared, ZEND_ACC_PUBLIC)
PHP_MALIAS(Phalcon_DI, offsetSet, setShared, arginfo_phalcon_di_setshared, ZEND_ACC_PUBLIC)

Zend will create both methods (any of them can be overridden by the descendants if necessary) but they will share the same implementation.

The advantage is that offsetSet() will be as fast as setShared() (no overhead for the call to the userland and no need to allocate a memory frame).

For the developers we may add something like:

/**
 * @brief This method is an alias for Phalcon\Di::setShared()
 * @see setShared()
 */
PHALCON_DOC_METHOD(Phalcon_Di, offsetSet);

What do you think?

@phalcon
Copy link
Collaborator

phalcon commented Jul 12, 2013

Looks great!

@ghost
Copy link
Author

ghost commented Jul 13, 2013

Done.

phalcon pushed a commit that referenced this pull request Jul 13, 2013
@phalcon phalcon merged commit 1c0552f into phalcon:1.2.1 Jul 13, 2013
@ghost ghost deleted the mm branch July 14, 2013 00:40
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.

2 participants