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

huge speedup for executing SQL with lots of bind variables #115

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from

Conversation

GitMensch
Copy link
Contributor

@GitMensch GitMensch commented Apr 25, 2024

dblib/ocesql.c (init_sql_var_list, reset_sql_var_list, add_sql_var_list, add_sql_res_var_list): remove the need to iterate over the complete SQLVARLIST of _sql_var_lists and _sql_res_var_lists by saving the pointer of the last entry, fixes #84

before this change a relative simple COBOL program creating a report from ~5000 DB reads had 60% of its cpu-cost in libocesql (mostly in the add_sql_res_var_list() function), while libpg had 6% (checked with perf record).

After this change liboceql is down to 12% and libpg using 16%

Setting to a draft for possible more performance-related follow-up changes.

@GitMensch GitMensch marked this pull request as draft April 25, 2024 22:04
@GitMensch

This comment was marked as outdated.

@n-isaka
Copy link
Member

n-isaka commented Apr 30, 2024

Thank you for the pull request. I don't have any particular problems and think it's a good change.

The problem seems to be that the definition for MAX_DIGITS is missing. Is it defined somewhere?

@GitMensch GitMensch marked this pull request as ready for review October 21, 2024 08:13
@GitMensch
Copy link
Contributor Author

Pushed with included changes to COMP-3 and DISPLAY / NATIONAL, now including MAX_DIGITS (which in my local version was much older), thanks to the GH action that is now enabled and the gitpod integration it was also possible to run the tests.

ocesql.c (init_sql_var_list, reset_sql_var_list, add_sql_var_list, add_sql_res_var_list):
remove the need to iterate over the complete SQLVARLIST of _sql_var_lists and _sql_res_var_lists by saving the pointer of the last entry, leading to huge speedup for executing SQL with lots of bind variables (nearly all time for binding was spent here before)
…memset instead of free + malloc)

This drops the amount memory allocation/free to the maximum amount of binding variables used.
A run of 8 seconds doing 5000 DB operations previously had 1,945,796 allocations via new_sql_var() and is now down to 426, reducing instructions and cycles executed in the test case by 20% as well.
* prefer local buffers over temporary mallocs
* only space-pad PIC X/PIC N instead of space-fill then overwrite
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.

performance issue - and easy fix
2 participants