Branch creation and performance tuning #2

NAL-6295 · 2025-06-28T05:07:23Z

No description provided.

…ive PHP algorithms Major optimizations implemented: - Replace custom binary search insertion with PHP usort() for OrderBy operations - Implement hash-based grouping for O(n) GroupBy performance - Optimize compare function with inlined logic and reduced overhead - Add batch processing for sorting operations - Eliminate redundant array operations (array_splice, array_push) - Memory optimizations with array shorthand syntax Performance improvements: - OrderBy: O(n²) → O(n log n) complexity - GroupBy: O(n²) → O(n) average case - 10,000 records OrderBy: ~40ms execution time - 2,000 records GroupBy: ~0.78ms execution time - All correctness tests passing Added comprehensive performance testing and documentation.

Copilot

Pull Request Overview

This PR introduces a performance testing harness and applies significant optimizations to the ArrayWrapper library to improve sorting and grouping performance.

Added performance_test.php with benchmarks for orderBy, groupBy, and combined operations
Expanded documentation in PERFORMANCE_REPORT.md to detail before/after complexity and results
Refactored ArrayWrapper.php to use native usort, hash-based grouping, inlined compare logic, batch processing, and shorthand array syntax

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File	Description
performance_test.php	New script to benchmark and verify optimized methods
PERFORMANCE_REPORT.md	Detailed report of performance issues and improvements
ArrayWrapper.php	Added `_orderByOptimized`, `_groupingOptimized`, simplified `compare`, batch processing, memory optimizations

Comments suppressed due to low confidence (3)

PERFORMANCE_REPORT.md:165

[nitpick] The phrase 'Exponential improvement' is misleading for a change from O(n²) to O(n log n); consider 'Significant improvement' or 'Log-linear improvement' instead.

| OrderBy | O(n²) | O(n log n) | Exponential improvement |

ArrayWrapper.php:1

New optimized methods (_orderByOptimized, _groupingOptimized, inlined compare) were added but lack dedicated unit tests; consider adding targeted tests to cover edge cases and ensure correctness.

<?php

ArrayWrapper.php:113

[nitpick] Concatenating with a literal '|' delimiter can cause hash collisions if values contain '|'. Consider using json_encode([$key1, $key2]) or a less common delimiter.

				$hashKey .= $value[$key] . '|';

Copilot · 2025-06-28T05:09:56Z

performance_test.php

+    $memoryUsed = ($endMemory - $startMemory) / 1024 / 1024; // Convert to MB
+
+    // Verify correctness
+    $isCorrect = (json_encode($expected) === json_encode($actual));


Comparing large arrays via json_encode can be slow and may introduce false negatives; consider using direct array comparison ($expected === $actual) or a specialized assertion for correctness checks.

Suggested change

$isCorrect = (json_encode($expected) === json_encode($actual));

$isCorrect = ($expected === $actual);

Copilot · 2025-06-28T05:09:57Z

performance_test.php

+    ))->toVar();
+
+    $endTime = microtime(true);
+    $endMemory = memory_get_usage();


Using memory_get_usage() only captures current usage; for more accurate peak memory measurement, consider memory_get_peak_usage() before and after the test.

Suggested change

$endMemory = memory_get_usage();

$endMemory = memory_get_peak_usage();

Copilot · 2025-06-28T05:09:57Z

ArrayWrapper.php

@@ -377,7 +316,7 @@ public function join($right,$leftKey,$rightKey,$map,$joinType = JoinType::INNER)
 			throw new Exception("$rightKey is not array");
 		}

-		if(!count($leftKey) == count($rightKey)){
+		if(count($leftKey) != count($rightKey)){
 			throw new Exception("$leftKey count diferrent $rightKey count.");


There's a typo in the exception message: 'diferrent' should be 'different'.

Suggested change

throw new Exception("$leftKey count diferrent $rightKey count.");

throw new Exception("$leftKey count different $rightKey count.");

cursor

Bug: Hash Key Vulnerabilities in `_groupingOptimized`

The _groupingOptimized method has two hash collision vulnerabilities. First, its hash key generation concatenates values using '|' without escaping, causing different groups to merge incorrectly if values contain the separator (e.g., key1="a|b", key2="c" and key1="a", key2="b|c" both hash to "a|b|c|"). Second, it only includes string and callable keys in the hash, silently ignoring other key types (e.g., integers). This omission can lead to incomplete hash keys and incorrect grouping of distinct data.

ArrayWrapper.php#L106-L124

ArrayWrapper/ArrayWrapper.php

Lines 106 to 124 in b238778

    
           	**/ 
        
           	private function _groupingOptimized(&$groups, $value, $groupKeys){ 
        
           		// グループキーのハッシュを生成 
        
           		$hashKey = ''; 
        
           		foreach($groupKeys as $keyInfo) { 
        
           			$key = $keyInfo[self::KEY]; 
        
           			if (is_string($key)) { 
        
           				$hashKey .= $value[$key] . '|'; 
        
           			} elseif (is_callable($key)) { 
        
           				$hashKey .= $key($value) . '|'; 
        
           			} 
        
           		} 
        
           		if (isset($groups[$hashKey])) { 
        
           			$groups[$hashKey][self::GROUP_VALUES][] = $value; 
        
           		} else { 
        
           			$groups[$hashKey] = $this->_addNewGroup($groupKeys, $value); 
        
           		} 
        
           	}

Fix in Cursor

Bug: Sorting Issues: Mixed Comparisons & Inverted DESC Logic

The compare method contains two bugs:

It mixes strict (!==) and loose (>) comparisons, leading to incorrect sorting for mixed data types (e.g., 5 and "5").
The DESC (descending) flag logic is inverted, causing orderBy operations to sort in the opposite direction than intended.

ArrayWrapper.php#L80-L87

ArrayWrapper/ArrayWrapper.php

Lines 80 to 87 in b238778

    
           if ($leftValue !== $rightValue) { 
        
           	if ($leftValue > $rightValue) { 
        
           		return $leftKeyInfo[self::DESC] ? -1 : 1; 
        
           	} else { 
        
           		return $leftKeyInfo[self::DESC] ? 1 : -1; 
        
           	} 
        
           }

Fix in Cursor

BugBot free trial expires on July 22, 2025
You have used $0.00 of your $0.01 spend limit so far. Manage your spend limit in the Cursor dashboard.

Was this report helpful? Give feedback by reacting with 👍 or 👎

- Modified average() method to use _source directly instead of toVar() - Fixed compatibility issue with GroupBy->Select->Average chain - All 15 original tests now pass successfully - Huge data test (10,000 records) completes in ~40ms with correct results

Co-authored-by: naruhiron <naruhiron@gmail.com>

cursoragent added 2 commits June 28, 2025 05:04

🧹 Clean up temporary development files

b238778

NAL-6295 requested a review from Copilot June 28, 2025 05:08

Copilot AI reviewed Jun 28, 2025

View reviewed changes

cursor bot reviewed Jun 28, 2025

View reviewed changes

cursoragent and others added 2 commits June 28, 2025 05:23

Optimize ArrayWrapper performance with O(n log n) sorting and bug fixes

1ac2cb5

Co-authored-by: naruhiron <naruhiron@gmail.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Branch creation and performance tuning #2

Branch creation and performance tuning #2

Uh oh!

NAL-6295 commented Jun 28, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Jun 28, 2025

Uh oh!

Copilot AI Jun 28, 2025

Uh oh!

Copilot AI Jun 28, 2025

Uh oh!

cursor bot left a comment

Uh oh!

Uh oh!

	$isCorrect = (json_encode($expected) === json_encode($actual));
	$isCorrect = ($expected === $actual);

	$endMemory = memory_get_usage();
	$endMemory = memory_get_peak_usage();

	throw new Exception("$leftKey count diferrent $rightKey count.");
	throw new Exception("$leftKey count different $rightKey count.");

	**/
	private function _groupingOptimized(&$groups, $value, $groupKeys){
	// グループキーのハッシュを生成
	$hashKey = '';
	foreach($groupKeys as $keyInfo) {
	$key = $keyInfo[self::KEY];
	if (is_string($key)) {
	$hashKey .= $value[$key] . '\|';
	} elseif (is_callable($key)) {
	$hashKey .= $key($value) . '\|';
	}
	}

	if (isset($groups[$hashKey])) {
	$groups[$hashKey][self::GROUP_VALUES][] = $value;
	} else {
	$groups[$hashKey] = $this->_addNewGroup($groupKeys, $value);
	}
	}


	if ($leftValue !== $rightValue) {
	if ($leftValue > $rightValue) {
	return $leftKeyInfo[self::DESC] ? -1 : 1;
	} else {
	return $leftKeyInfo[self::DESC] ? 1 : -1;
	}
	}

Branch creation and performance tuning #2

Are you sure you want to change the base?

Branch creation and performance tuning #2

Uh oh!

Conversation

NAL-6295 commented Jun 28, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Jun 28, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jun 28, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jun 28, 2025

Choose a reason for hiding this comment

Uh oh!

cursor bot left a comment

Choose a reason for hiding this comment

Bug: Hash Key Vulnerabilities in `_groupingOptimized`

Bug: Sorting Issues: Mixed Comparisons & Inverted DESC Logic

Uh oh!

Uh oh!