Skip to content

Fix performance degradation for bulk inserts #2354

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

Merged
merged 1 commit into from
May 4, 2025
Merged

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