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

More performance, speed first #16005

Open
williamdes opened this issue Mar 3, 2020 · 9 comments
Open

More performance, speed first #16005

williamdes opened this issue Mar 3, 2020 · 9 comments
Assignees
Labels
enhancement A feature request for improving phpMyAdmin

Comments

@williamdes
Copy link
Member

williamdes commented Mar 3, 2020

This issue focuses on tracking performance issues and improvements that can be made.

You can use xdebug or xhprof. Xhprof is just better. Here is how to use it.

xdebug config

xdebug.profiler_enable=0
xdebug.profiler_enable_trigger=1
xdebug.profiler_append=1
xdebug.profiler_output_dir="/mnt/Dev/@phpmyadmin/theREALphpMyAdminREPO"
xdebug.profiler_output_nme="xdebug-profile.%t.%p"

xdebug.auto_trace=1
xdebug.trace_enable_trigger=1
xdebug.trace_options=1
xdebug.collect_params=4
xdebug.collect_return=0
xdebug.trace_format=0
xdebug.show_mem_delta=1
xdebug.trace_output_dir="/mnt/Dev/@phpmyadmin/theREALphpMyAdminREPO"
xdebug.trace_output_name="trace.%t.%p"

xdebug.file_link_format="xdebug://%f@%l"
xdebug.remote_log="/mnt/Dev/@phpmyadmin/theREALphpMyAdminREPO/xdebug.log"

Here we are talking about milliseconds. This is the point of this issue.

@williamdes williamdes added the enhancement A feature request for improving phpMyAdmin label Mar 3, 2020
@williamdes
Copy link
Member Author

@MauricioFauth the DI yaml parsing is very very complicated and uses a lot of operations.
I would want to change it to a format that does not need complex parsing
Do you agree ?

@MauricioFauth
Copy link
Member

If you think it is a good improvement, go ahead. And if I remember correctly, this is the only place where we use the yaml dependency.

@williamdes
Copy link
Member Author

I am working on it, kind of complicated because I need to write some php code to process the array but I will end up making it work

@MauricioFauth
Copy link
Member

MauricioFauth commented Mar 4, 2020

Isn't it easier to switch the file loader?

@williamdes
Copy link
Member Author

The XML parsing is better than the yaml one but using a php array in a file should even better

williamdes added a commit to williamdes/phpmyadmintest that referenced this issue Mar 4, 2020
Signed-off-by: William Desportes <williamdes@wdes.fr>
@williamdes
Copy link
Member Author

Done in #16007

williamdes added a commit to williamdes/phpmyadmintest that referenced this issue Mar 4, 2020
Signed-off-by: William Desportes <williamdes@wdes.fr>
williamdes added a commit to williamdes/phpmyadmintest that referenced this issue Mar 5, 2020
Signed-off-by: William Desportes <williamdes@wdes.fr>
williamdes added a commit that referenced this issue Mar 5, 2020
…removing yaml parsing

Pull-request: #15678
Ref: #16005

Signed-off-by: William Desportes <williamdes@wdes.fr>
williamdes added a commit that referenced this issue Mar 5, 2020
Yes, I added 2 entries. I want to make it obvious that we removed a dependency.
Packaging folks ;)

Signed-off-by: William Desportes <williamdes@wdes.fr>
williamdes added a commit to williamdes/phpmyadmintest that referenced this issue Mar 19, 2020
Pull-request: phpmyadmin#16031
Ref: phpmyadmin#16005
Signed-off-by: William Desportes <williamdes@wdes.fr>
williamdes added a commit that referenced this issue Mar 28, 2020
Ref: #16005
Pull-request: #16028
Signed-off-by: William Desportes <williamdes@wdes.fr>
williamdes added a commit that referenced this issue Mar 28, 2020
Pull-request: #16028
Signed-off-by: William Desportes <williamdes@wdes.fr>
williamdes added a commit that referenced this issue Jun 14, 2020
Pull-request: #16031
Ref: #16005
Ref: f577a71

Signed-off-by: William Desportes <williamdes@wdes.fr>
@williamdes williamdes added this to Triage zone in Enhancements via automation Jul 23, 2020
@williamdes williamdes moved this from Triage zone to Code base in Enhancements Jul 23, 2020
@williamdes williamdes removed this from Code base in Enhancements Jul 24, 2020
@williamdes williamdes added this to Triage zone in Enhancements via automation Jul 24, 2020
@williamdes williamdes moved this from Triage zone to Code base in Enhancements Jul 24, 2020
@williamdes williamdes self-assigned this Jan 28, 2021
williamdes added a commit that referenced this issue Jan 28, 2021
Pull-request: #16593
Ref: #16005

Signed-off-by: William Desportes <williamdes@wdes.fr>
williamdes added a commit that referenced this issue Jan 28, 2021
Signed-off-by: William Desportes <williamdes@wdes.fr>
@williamdes
Copy link
Member Author

This patch did slow down the export by A LOT: e61b213

williamdes added a commit that referenced this issue Feb 25, 2021
Ref: #16005
Pull-request: #16651

Signed-off-by: William Desportes <williamdes@wdes.fr>
@williamdes
Copy link
Member Author

williamdes commented Mar 4, 2021

Performance finding: calling PHP getters a LOT of times (example: 50000 times) is not worth it. Use a public class attribute instead (and add @readonly as a phpdoc).

@NKarasek
Copy link

NKarasek commented Mar 4, 2021

Regarding good coding practice - and optimization. In looking through the PHPMyAdmin file 'move.js', I found many places where code developed a reference to an object using 'getElementById()'. While doing this once or twice is not a big deal, doing it within a loop (worse - multiple loops - perhaps looking at ALL of the columns for ALL of the tables available to Designer for example) is a big deal. To optimize this scenario, it would be considerably better to develop the necessary reference to the element (object), store that reference in a local var, and use the var in further calls to object attributes (for example, or adding/removing classes, css attributes, everything.)

I saw a bug report a few months ago that called out designer for glitching during the drag of a table through a designer page - a common enough scenario. I replicated the problem on my (admittedly old - read Sssllllooowww) Dell system and solved the problem by rewriting everything I could find in move.js that met the scenario above. I only addressed cases where calls to 'getElementById()' occurred more than two times in a row - the break-even point when computing Big 'O' (without delving into exactly how JQuery works its magic :-)

As a general rule - just my suggestion - but I would recommend that this type of optimization be a part of all 'best coding' practices - certainly with a project as deep and wide as PHPMyAdmin. I will be checking out other js files and templates to see if this concept can be used to advantage elsewhere.

williamdes added a commit to williamdes/phpmyadmintest that referenced this issue Dec 12, 2021
Signed-off-by: William Desportes <williamdes@wdes.fr>
williamdes added a commit to williamdes/phpmyadmintest that referenced this issue Dec 12, 2021
Signed-off-by: William Desportes <williamdes@wdes.fr>
williamdes added a commit that referenced this issue Dec 19, 2021
…ort pages

Pull-request: #17242
Ref: #16005

Signed-off-by: William Desportes <williamdes@wdes.fr>
williamdes added a commit that referenced this issue Dec 19, 2021
Pull-request: #17242
Ref: #16005
Ref: 78fd92f

Signed-off-by: William Desportes <williamdes@wdes.fr>
williamdes added a commit that referenced this issue Jul 14, 2023
Signed-off-by: William Desportes <williamdes@wdes.fr>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement A feature request for improving phpMyAdmin
Projects
Enhancements
  
Code base
Development

No branches or pull requests

3 participants