Skip to content

Conversation

@MasterOdin
Copy link
Member

@MasterOdin MasterOdin commented May 4, 2025

PR fixes performance degradation for bulk inserts that was introduced in #2348. The issue was that for the new function I introduced (getInsertParameters), we were doing an array_merge to append its return value to our building list of parameters. While this worked, as we were making a copy of the $params array for each row to be inserted, for enough rows, it would become very slow. By using pass by reference, and just appending to the same array throughout, managed to achieve a great speed-up, while maintaining the DRY intention of getInsertParameters.

By moving to a pass by reference setup, given the following migration:

<?php

declare(strict_types=1);

use Phinx\Migration\AbstractMigration;

final class Numbers extends AbstractMigration
{
    /**
     * Change Method.
     *
     * Write your reversible migrations using this method.
     *
     * More information on writing migrations is available here:
     * https://book.cakephp.org/phinx/0/en/migrations.html#the-change-method
     *
     * Remember to call "create()" or "update()" and NOT "save()" when working
     * with the Table class.
     */
    public function change(): void
    {
        $table = $this->table('numbers')
            ->addColumn('key', 'integer')
            ->addColumn('value', 'integer');
        $table->create();

        if ($this->isMigratingUp) {
            for ($i = 1; $i < 120000; $i++) {
                $table->insert([
                    'key' => $i,
                    'value' => $i
                ]);
            }
            $table->saveData();
        }
    }
}

I was able to reduce the time it took for sqlite to run this from 41.8165s to 0.3525s.

@dereuromark dereuromark merged commit 454d303 into 0.x May 4, 2025
12 checks passed
@dereuromark dereuromark deleted the fix-bulk-insert branch May 4, 2025 16:14
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