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

Immutable classes with pure constructor and same constant args as single object #754

Open
mkornaukhov03 opened this issue Feb 21, 2023 · 6 comments
Assignees
Labels
enhancement New feature or request optimization Memory comsumption / CPU speedup

Comments

@mkornaukhov03
Copy link
Contributor

mkornaukhov03 commented Feb 21, 2023

In case of

$a = new A(CONST_ONE, CONST_TWO);
$b = new A(CONST_ONE, CONST_TWO);

we want

  1. Constructor of class A with parameters CONST_ONE, CONST_TWO to be called once and transpiled as global variable that is initialized only once
  2. Each invocation of new A(CONST_ONE, CONST_TWO) to be replaced with such a variable

It would allow use some constructions such as

const c = new A(CONST_ONE, CONST_TWO);

with the following restrictions on class A:

  • Marked as @kphp-immutable-class
  • Has constructor that marked as @kphp-pure-function
  • CONST_ONE, CONST_TWO are constant expressions

It would possibly improve performance and definitely help to implement enums in a natural way.

@mkornaukhov03 mkornaukhov03 added enhancement New feature or request optimization Memory comsumption / CPU speedup labels Feb 21, 2023
@mkornaukhov03 mkornaukhov03 self-assigned this Feb 21, 2023
@vkaverin
Copy link

vkaverin commented Feb 26, 2023

I strongly disagree about this change. Two independent calls of the constructor MUST return two independent instances, but suggested feature just breaks this rule and affects runtime.

In fact, this issue introduces some kind of compile-time singleton, but in a very implicit way (counterintuitive, in fact). Two identical objects, even immutable ones, are still not the same object in general case, so singletonization must be done in user code explicitly.

So this in not an optimization, but a breaking change.

@mkornaukhov03
Copy link
Contributor Author

I strongly disagree about this change. Two independent calls of the constructor MUST return two independent instances, but suggested feature just breaks this rule and affects runtime.

In fact, this issue introduces some kind of compile-time singleton, but in a very implicit way (counterintuitive, in fact). Two identical objects, even immutable ones, are still not the same object in general case, so singletonization must be done in user code explicitly.

So this in not an optimization, but a breaking change.

Thank you for the comment. Naturally, it's internal compiler change and it should not affect on current code bases because all constructors are not @kphp-pure-function-ed. According to the idea, it doesn't matter for kphp-user how such classes are transpiled.

Could you give an example of breaking cases, please?

@vkaverin
Copy link

Could you give an example of breaking cases, please?

I missed the fact that @kphp-pure-function is currently supported only for built-in functions, so you're right, current behaviour won't change. Is the issue about implementing the annotation for constructors (and other user-code functions) as well?


By the way, such behaviour can't be achieved in pure PHP, so there will be inconsistency between PHP and KPHP.

@mkornaukhov03
Copy link
Contributor Author

mkornaukhov03 commented Feb 27, 2023

Is the issue about implementing the annotation for constructors (and other user-code functions) as well?

Not really. Actually, this annotation is implemented, but behavior is not guaranteed. There's going to be another issue about dealing with @kphp-pure-function.

For now, the most motivated example is enum implementation.

@vkaverin
Copy link

vkaverin commented Mar 1, 2023

Do you mean it's gonna be kind of workaround while there's no real enum, like for PHP <= 8.1? Or this blocks #730 in some way?

@mkornaukhov03
Copy link
Contributor Author

Do you mean it's gonna be kind of workaround while there's no real enum, like for PHP <= 8.1? Or this blocks #730 in some way?

We decided to change a little bit initial idea. For now, we implemented a part of https://www.php.net/releases/8.1/en.php#new_in_initializers, namely it's possible to use objects in constant definition. See #814.
Besides, we decided to (later) introduce new phpdoc for classes that forces to store every instance of this class which are constructed with same constant parameters in the same constant object.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request optimization Memory comsumption / CPU speedup
Projects
None yet
Development

No branches or pull requests

2 participants