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

PHP 7.2 Deprecations: create_function #25

Open
fandrieu opened this issue Feb 15, 2018 · 34 comments
Open

PHP 7.2 Deprecations: create_function #25

fandrieu opened this issue Feb 15, 2018 · 34 comments
Assignees
Labels
type:improvement Improvement

Comments

@fandrieu
Copy link

As as follow-up to #24: the create_function has been deprecated in php 7.2.

Would it be possible to update Utils::createLambdaFromString to avoid using that function ?

Thanks

@Athari Athari added the type:improvement Improvement label Feb 15, 2018
@Athari
Copy link
Owner

Athari commented Feb 15, 2018

The only alternative to create_function is eval which doesn't allow caching of compiled code, so causes considerably worse performance if the same string lambda is called more than once with different arguments. This deprecation doesn't make any sense to me, it just removes a feature for the sake of removing a feature. The reasoning given in the manual is pure bullshit.

My advice would be:

  1. If you want to preserve the performance provided by caching of evaluated string lambdas and find the performance hit of string lambdas acceptable, you should disable deprecation warnings in php.ini.

  2. If you want maximum performance available with YaLinqo, or you can't modify PHP's deprecation settings, you should follow the advice given by PHP manual and use anonymous functions.

In either case, if you know a way to convince stupid bullheaded PHP devs to finally implement damn arrow functions which has been in RFC for ages and has been requested since forever, please do so. If these retarded fucks finally implement arrow functions, I can finally drop support for string lambdas and forget this abomination ever existed.

If I understand the deprecation process correctly, leaving create_function in the code doesn't cause any real issues right now, but will when the deprecated function is eventually fully removed in the next major PHP release. When that happens, I'll switch to using eval, or drop support for string lambdas completely if arrow functions are implemented. I'll keep the issue open to keep track of the status of create_function.

Note that some string lambdas like '$v' or '$k' are cached internally by YaLinqo, so you can keep using them in place of more verbose static methods of the Functions class and even more verbose anonymous functions without causing deprecation warning to be emitted by PHP.

@sanmai
Copy link
Contributor

sanmai commented Mar 17, 2018

The benchmark shows that is slightly faster to create a function with create_function than use regular closures. This could be a reason why YaLinqo is faster when string functions are used. So there could be a performance penalty if create_function will be removed.

@Bilge
Copy link

Bilge commented Dec 9, 2018

The create_function invocation has been silenced (@) in master via 7591c4d and #30, but there is still no tag to accommodate the change, so all projects using YaLinqo fail on PHP 7.2 with Function create_function() is deprecated. @Athari Can we get a new tag?

@Athari
Copy link
Owner

Athari commented Dec 12, 2018

@Bilge You have a point. I'll add a minor 2.x version I think, as master is currently unfinished 3.0.

Hm, PHP 7.3 is available. Thank gods, they didn't remove deprecated functions. That would happen only in 8.0, right? I hope that by that time, these idiots will finally add arrow functions instead of implementing random garbage like instanceof on literals and numeric keys in objects. Ugh.

Hm, looks like I need to remove support for real type. Will be removed in 7.4.

@Bilge
Copy link

Bilge commented Dec 12, 2018

Yes, they would never remove any deprecated functions in a minor release. But that's besides the point, the deprecation warnings themselves are the problem in this case.

@TomasVotruba
Copy link

If you need to upgrade more than one create_function to anonymous function (official recommended upgrade), you can use a CLI tool for that.

It's tested on 30 various (and really weird :)) cases.

-$callback = create_function('$a', 'return "<cas:proxy>$a</cas:proxy>";');    
+$callback = function ($a) {
+    return "<cas:proxy>{$a}</cas:proxy>";
+};

Includes concat (.), string quotes and inclined function calls:

-$func = create_function('$atts, $content = null','return "<div class=\"' . $class_list . '\">" . do_shortcode($content) . "</div>";' );
+$func = function ($atts, $content = null) use ($class_list) {
+    return "<div class=\"{$class_list}\">" . do_shortcode($content) . "</div>";
+};

Do you want to automate the hard work?

1. Instal Rector

composer require rector/rector --dev

2. Create config

# rector.yml
services:
    Rector\Php\Rector\FuncCall\CreateFunctionToAnonymousFunctionRector: ~

3. Upgrade your Code

vendor/bin/rector process src --config rector.yml --dry-run
vendor/bin/rector process src --config rector.yml

Enjoy!

@Bilge
Copy link

Bilge commented Jan 6, 2019

@Athari It's been a month since I raised this. Any update on the minor tag?

Athari added a commit that referenced this issue Jan 10, 2019
@Athari
Copy link
Owner

Athari commented Jan 10, 2019

@Bilge Back-ported changes from master which affect supported PHP versions, added tag v2.4.2. I may have broken something in the process (messed up rebase, had to reset branches), so please test.

sanmai added a commit to sanmai/YaLinqo that referenced this issue Jan 11, 2019
-  PHP versions below 7.0 are not tested under, should not be shown as supported.
- php-coveralls now has is a different package

See  Athari#25
@sanmai
Copy link
Contributor

sanmai commented Jan 11, 2019

Looking at a0e782a I can say there's one thing missing surely. If you don't test on PHP 5.6, do not declare support for it. #43 should fix that problem, if there is a problem (I'm not entirely sure what went where, GitHub isn't helping).

(CI's failing on this commit. I think I've fixed these non-numeric values before, should be easy to cherry-pick. Though master build is all green, so guess it's all right.)

@sanmai
Copy link
Contributor

sanmai commented Jan 11, 2019

@TomasVotruba IIRC create_function was faster on benchmarks than a modern anonymous function.

@TomasVotruba
Copy link

How is that relevant in case of removed function?

@sanmai
Copy link
Contributor

sanmai commented Jan 11, 2019

Sure, when it's removed, it's removed. When it's not, @create_function is not as bad. (I'd run another benchmark to see how worse it is with an at mark, though.)

@TomasVotruba
Copy link

Piling up deprecations didn't proove as good practice. Better solve 5 deprecations per minor version, than 20 at once.

@Athari Athari self-assigned this Mar 3, 2019
@andre719mv
Copy link

andre719mv commented Aug 31, 2019

Any news on this? I am still getting a lot of warnings on v2.4.2.

@sanmai
Copy link
Contributor

sanmai commented Sep 2, 2019

Shall we hard-fork this?

@Athari
Copy link
Owner

Athari commented Sep 2, 2019

@andre719mv @sanmai What is your problem exactly? The code works. The error is muted. If you don't use string lambdas, the deprecated function in never executed.

Where do you get "a lot of warnings" from? Especially in plural form. From analysis tools?

PHP 7.4 still hasn't been released. I don't see any benefits of releasing "YaLinqo 3.0" with the only feature being "removed feature" to satisfy a deprecation in a minor release of PHP 7.2, just to release YaLinqo 4.0 a month later with proper argument type hints which would make sense in PHP 7.4 supporting arrow functions.

If you prefer to inconvenience yourself with forks instead of configuring your analysis tools, feel free to do so. In the meantime, I can offer adding a branch where create_function is replaced with throwing an exception, so you can switch to that instead of a named package version, if that's what you're looking for. However, if you give me one good reason to bump a major version number just for deprecation, I'll reconsider my plans of making YaLinqo 3.0 a version switching to stricter argument typing, dropping string lambdas in favor of arrow functions and making a couple of long overdue breaking changes (#41, #11 etc.).

@andre719mv
Copy link

Hi!
Yes, I have some analysis tool on my site to monitor all errors and warnings. Warnings from this lib has filled a lot of space and it`s hard to see other problems.
image

Thanks for explanation. Now I see that there is no good way to fix it before PHP 7.4. No problem. I`ll add exception to analysis tool.

@Bilge
Copy link

Bilge commented Sep 2, 2019 via email

@sanmai
Copy link
Contributor

sanmai commented Sep 4, 2019

And warnings are not. Here's a benchmark I cooked from a thin air:

$start = microtime(true);

for ($i = 0; $i < 1000; $i++) {
    $fn = @create_function('$a,$b', 'return log($a * $b);');
}

var_dump(microtime(true) - $start);

$start = microtime(true);

for ($i = 0; $i < 1000; $i++) {
    $fn = function ($a, $b) {return log($a * $b);};
}

var_dump(microtime(true) - $start);

Code with create_function is about 100 times slower because of the muted warning.

Output for 7.3.9
float(0.015002012252808)
float(0.00026297569274902)

See for yourself: https://3v4l.org/13afF

If create_function was faster before, not anymore now.

@Athari
Copy link
Owner

Athari commented Sep 4, 2019

@sanmai
Your synthetic test has nothing in common with how the library works. The function create_function allows caching of the compiled result, so you need to call it once per each unique string lambda. On the other hand, the not yet deprecated function eval forces you to compile the function for every single call, no matter whether it's unique or not. On top of this, it also does not support arguments, so you're forced to serialize values into code.

Let's run a test which reflects how the library actually works:

<?php

$start = microtime(true);
$fn = @create_function('$a,$b', 'return log($a * $b);');
$v = 0;
for ($i = 0; $i < 1000; $i++)
    $v += $fn($i, $i);
echo("create_function: " . (microtime(true) - $start) . "\n");

$start = microtime(true);
$v = 0;
for ($i = 0; $i < 1000; $i++)
    $v += eval("return log($i * $i);");
echo("eval: " . (microtime(true) - $start) . "\n");

$start = microtime(true);
$fn = fn($a, $b) => log($a * $b);
$v = 0;
for ($i = 0; $i < 1000; $i++)
    $v += $fn($i, $i);
echo("arrow function: " . (microtime(true) - $start) . "\n");

$start = microtime(true);
$fn = function ($a, $b) { return log($a * $b); };
$v = 0;
for ($i = 0; $i < 1000; $i++)
    $v += $fn($i, $i);
echo("anonymous function: " . (microtime(true) - $start) . "\n");

Output:

create_function: 0.00020408630371094
eval: 0.0031659603118896
arrow function: 0.00015783309936523
anonymous function: 0.00012993812561035

As you can see, the "eval" version is horrendously slow. "Fixing" the deprecation warning by following the suggestion from the developers of PHP and replacing create_function with eval would make the library 15x times slower. It would also be a major breaking change as very few values can be easily serialized for eval. Obviously, this is not an option, as it would make the library unusable for anyone who relies on string lambdas.

The "create_function" version is still slower than anonymous and arrow functions (1.5x), of course, but it's a known compromise between performance and conciseness. YaLinqo as a whole is a compromise between performance and readability as you do lose performance compared to direct built-in function calls.

It's typical for developers of PHP to break things before fixing and then break them again. In this case, they decided to deprecate a function before providing a valid alternative. PHP 7.4 has breaking changes too, which aren't suited for a minor version release, and it breaks more than a dozen of top packages. That's just how PHP is developed.

@andre719mv
Copy link

Kind reminder =).
PHP 7.4 has been released on November 28, 2019

@Athari
Copy link
Owner

Athari commented Feb 20, 2020

Working on it.

@vspl-girish
Copy link

With PHP 8.0 release, this is broken! The code no longer works.

@seiz
Copy link

seiz commented Feb 18, 2021

+1 for fixing this to gain PHP8 compatibility

@Bilge
Copy link

Bilge commented Feb 18, 2021

This library is abandoned.

@UweKeim
Copy link

UweKeim commented Mar 7, 2021

Although the library seems to be dead, I successfully managed to convert the few calls in my code that triggered the usage of create_function.

This was before:

$r->OptionValues = 
    E::from($model->Options)->select('$v->Value')->toArray();

And this is how I migrated it to:

$r->OptionValues = 
    E::from($model->Options)->select(function ($v) {return $v->Value;})->toArray();

I.e. I replaced the string with my code, that was converted by the YaLinqo to a function by simply providing the anonymous function by myself.

As of now, this seems to work just perfectly. Hopefully it stays this way.

@vspl-girish
Copy link

Thanks for the workaround. Successfully implemented it and got our code to work!

@jorrit
Copy link

jorrit commented Mar 8, 2021

I think that that is the only solution going forward. By the way, you can shorten the lamdba function by using fn ($v) => $v->Value.

@UweKeim
Copy link

UweKeim commented Mar 8, 2021

@jorrit Is this "short syntax" compatible with older PHP versions?

According to https://www.php.net/manual/en/functions.arrow.php this only works with PHP 7.4 and newer.

Unfortunately I have to support older versions, too.

@jorrit
Copy link

jorrit commented Mar 8, 2021

You're right about that. My suggestion is just for codebases that only need to support 7.4 and up.

@flashover
Copy link

Can confirm, using an anonymous function within Yalinqo from($array)->sum(fn(x) => $x->getSomeValue()); works but will there be an official update in this repository for PHP 8? I don't have a need for caching / performance.

Athari added a commit that referenced this issue Jul 11, 2023
… with eval.

Fixed PHPUnit dependency (PHP_Timer).
@Athari
Copy link
Owner

Athari commented Jul 11, 2023

Latest news

  1. I updated v2.0 branch with commit tagged v2.5.0. It replaces create_function call with eval call.
  2. My assumtions above about performance issues can be ignored as I assumed some sort of eval($code) while eval("return function($args) { $code };") does what I want exactly how I want.
  3. I haven't used PHP for ages and have no idea what I'm doing. I pushed the code and added the tag. Unit tests seem to work. Composer should notice the update, right? Let me know whether any of this is in any way functional.
  4. The "3.0 massive performance upgrade" is abandoned forever. I failed to implement any remotely useful optimizations in any remotely meaningful logic paths. Maybe it's impossible, maybe I lack the required IQ. Either way, it won't happen.

Future plans

The master branch does contain some useful changes besides failed optimization (most of it isn't even in the repo), including a few extra methods and API fixes (with breaking changes). At the moment I'm considering releasing everything as 3.0. Maybe with extra callable type hints and whataver else newest PHP contains, thus dropping "string lambdas" functionality.

Considering nobody seems to be interested in new functionality, including me, the difference between 2.0 and 3.0 will be 5% cleaner API, 3 extra methods, dropped string lambdas and PHP 8+ requirement.

Let me know what you think. Assuming anyone is still following this issue.

@jorrit
Copy link

jorrit commented Jul 11, 2023

I think steady maintenance is exactly what this library needs. Just keeping up with the changes in PHP and merging the occasional contribution seems like a good plan. Thanks for your efforts!

@Athari
Copy link
Owner

Athari commented Jul 12, 2023

Apparently Packagist requires a hook now. It failed to install one automatically, so I added it manually. Seems to work now, the 2.5 version of the package is up.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:improvement Improvement
Projects
None yet
Development

No branches or pull requests