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

Inline escape() method to improve performance #34

Merged
merged 4 commits into from
Mar 3, 2024

Conversation

staabm
Copy link

@staabm staabm commented Feb 20, 2024

dumping a InnoDB table containing 1,4 GiB of data ...

<?php // test.php

error_reporting(E_ALL);

require_once __DIR__ . '/vendor/autoload.php';

use Druidfi\Mysqldump as IMysqldump;

$date = date('Ymd');
$dumpSettings = array(
    'no-data' => false,
    'add-drop-table' => true,
    'single-transaction' => true,
    'lock-tables' => false,
    'add-locks' => true,
    'extended-insert' => true,
    'disable-foreign-keys-check' => true,
    'skip-triggers' => false,
    'add-drop-trigger' => true,
    'databases' => true,
    'add-drop-database' => true,
    'hex-blob' => true,
    'include-tables' => array('my-table'), // add a table name here
);

$dump = new IMysqldump\Mysqldump("mysql:host=$hostname;dbname=$db", $username, $password, $dumpSettings); // add DB credentials
$dump->start("backup$date.sql");

before this change

mstaab@mst22:/cluster/www/www/www/mysqldump-php$ time php test.php

real    0m41.353s
user    0m30.015s
sys     0m7.903s
mstaab@mst22:/cluster/www/www/www/mysqldump-php$ time php test.php

real    0m43.333s
user    0m31.603s
sys     0m8.279s
mstaab@mst22:/cluster/www/www/www/mysqldump-php$ time php test.php

after this change

mstaab@mst22:/cluster/www/www/www/mysqldump-php$ time php test.php

real    0m37.625s
user    0m26.760s
sys     0m7.848s

mstaab@mst22:/cluster/www/www/www/mysqldump-php$ time php test.php

real    0m37.937s
user    0m26.506s
sys     0m8.259s

the improvement works, because the escape method is called for every column in every row, so the method call overhead is visible

grafik

PHP Version

PHP 8.1.27

Operating System

ubuntu 22 lts

@staabm staabm changed the base branch from 2.x to master February 20, 2024 13:09
@staabm staabm changed the base branch from master to main February 20, 2024 13:09
src/Mysqldump.php Outdated Show resolved Hide resolved
Co-authored-by: Kamil Tekiela <[email protected]>
@back-2-95
Copy link
Member

Hi @staabm ! Thanks for the PRs! ❤️

I'll try to find time to go through them in coming weeks (I just got back from holiday).
ALSO I checked your GDPR tools and I would be happy to replace our current toolchain with yours.

@staabm
Copy link
Author

staabm commented Feb 24, 2024

hey 👋!

Take your time.

ALSO I checked your GDPR tools and I would be happy to replace our current toolchain with yours.

Not sure what you mean. Feel free to ping me if I can help you

@back-2-95
Copy link
Member

Aah, sorry. I meant this tool which I found via your profile: https://github.com/Smile-SA/gdpr-dump
I see now that you have made some contribution there AND that tools uses this library 😍

@staabm
Copy link
Author

staabm commented Feb 24, 2024

I came to your project here via said library which I started using a few weeks ago :-)

My motivation for improving performance here is making the gdpr lib faster :-)

@back-2-95 back-2-95 mentioned this pull request Feb 25, 2024
@back-2-95 back-2-95 added the enhancement New feature or request label Feb 25, 2024
@staabm
Copy link
Author

staabm commented Feb 29, 2024

committed another small micro optimization, which also improves readability a bit

@back-2-95 back-2-95 merged commit 2ececab into druidfi:main Mar 3, 2024
@staabm staabm deleted the inline3 branch March 3, 2024 12:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants