Improve performance of MemoryEfficientLongestCommonSubsequenceCalculator#118
Conversation
MemoryEfficientLongestCommonSubsequenceCalculator
|
thank you. I will have another look into |
|
Is this also the case for the v4? Many of use are stuck with PHPUnit 9 which requires diff ^4, can I do a PR for the v4? |
|
I think v4 is also affected. Sebastian needs to decide whether its worth backporting. |
|
Sure, I would consider a PR that backports these performance improvements to ^4. |
|
While in this case the performance fix is justified: It does not nearly have the practical effect shown in the profiler. On a standard 8.2 it is approximatively 3x as expensive (the empty loop takes 1.15 seconds on that machine). A profiler like blackfire traces every single function call and thus has a comparatively high overhead. On the PHP side a simple patch for diff --git a/ext/standard/array.c b/ext/standard/array.c
index 8a74704967..7d42a6ade2 100644
--- a/ext/standard/array.c
+++ b/ext/standard/array.c
@@ -1275,11 +1275,28 @@ PHP_FUNCTION(max)
int i;
max = &args[0];
+ if (Z_TYPE_P(max) == IS_LONG) {
+ zend_long max_lval = Z_LVAL_P(max);
- for (i = 1; i < argc; i++) {
- is_smaller_or_equal_function(&result, &args[i], max);
- if (Z_TYPE(result) == IS_FALSE) {
- max = &args[i];
+ for (i = 1; i < argc; i++) {
+ if (EXPECTED(Z_TYPE(args[i]) == IS_LONG)) {
+ if (max_lval < Z_LVAL(args[i])) {
+ max_lval = Z_LVAL(args[i]);
+ max = &args[i];
+ }
+ } else {
+ goto non_lval;
+ }
+ }
+
+ RETURN_LONG(max_lval);
+ } else {
+ for (i = 1; i < argc; i++) {
+non_lval:
+ is_smaller_or_equal_function(&result, &args[i], max);
+ if (Z_TYPE(result) == IS_FALSE) {
+ max = &args[i];
+ }
}
} |
|
@bwoebi thank you for getting into the discussion. could you do the necessary changes at php-src level? this would certainly help for a lot of applications arround. |
|
to make sure we don't loose bwoebis very valuable comment, I went ahead and created a php-src issue: php/php-src#11192 |
when creating a diff of a file with 5000 lines we can see
MemoryEfficientLongestCommonSubsequenceCalculatorto bottleneck onmax(). I think the main problem is the function call overhead, as its invoked ~43.000.000 times.with this change we just use plain php, which improves our workload in rector by 1 minute:
refs rectorphp/rector#7899