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

Remove useless calls to ProductOpener::Display::remove_tags_and_quote in Import.pm #2574

Closed
stephanegigandet opened this issue Nov 10, 2019 · 3 comments
Assignees
Labels
🚅 Performance Perl Related to the Perl code of the ProductOpener server

Comments

@stephanegigandet
Copy link
Contributor

While investigating #2563, @zigouras found there were tons of calls to ProductOpener::Display::remove_tags_and_quote when we run import_csv_file.pl

Many of those calls are done on values that do not have defined values. The checks are now done only on defined values, and they are inlined in one place (thus removing the calls).

@stephanegigandet stephanegigandet added the 🐛 bug This is a bug, not a feature request. label Nov 10, 2019
@stephanegigandet stephanegigandet self-assigned this Nov 10, 2019
@stephanegigandet
Copy link
Contributor Author

On a test import of 1000 products (with very few defined values):

Before: Profile of ./import_csv_file.pl for 52.4s (of 74.3s), executing 34385037 statements and 19654675 subroutine calls in 739 source files and 257 string evals.

After: Profile of ./import_csv_file.pl for 26.6s (of 35.6s), executing 12122014 statements and 2574614 subroutine calls in 739 source files and 255 string evals.

On the test import, there is a very significant speed improvement. The memory usage is almost the same though.

@hangy hangy added 🚅 Performance Perl Related to the Perl code of the ProductOpener server and removed 🐛 bug This is a bug, not a feature request. labels Nov 10, 2019
stephanegigandet added a commit that referenced this issue Nov 11, 2019
remove useless calls to remove_tags_and_quote(), bug #2574 #2563
@zigouras
Copy link
Contributor

While investigating #2563, @zigouras found there were tons of calls to ProductOpener::Display::remove_tags_and_quote when we run import_csv_file.pl

Many of those calls are done on values that do not have defined values. The checks are now done only on defined values, and they are inlined in one place (thus removing the calls).

Calling a function does not negatively impact performance by itself so putting the code inline will not make a difference. It is better practice to organize your code in functions.

@stephanegigandet
Copy link
Contributor Author

Calling a function does not negatively impact performance by itself so putting the code inline will not make a difference. It is better practice to organize your code in functions.

Unfortunately in Perl calling functions does add overhead for each call, so we have to strike a balance between readability / maintainability in performance. The inlining is worth it only for functions that get called a lot.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🚅 Performance Perl Related to the Perl code of the ProductOpener server
Projects
None yet
Development

No branches or pull requests

3 participants