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

db_text: new parameter: buffer_size #3352

Merged
merged 1 commit into from
Apr 11, 2024
Merged

db_text: new parameter: buffer_size #3352

merged 1 commit into from
Apr 11, 2024

Conversation

ovidiusas
Copy link
Member

Summary
New parameter for db_text: buffer_size.
This will allow db_text to handle larger text files.

@@ -125,6 +126,12 @@ dbt_table_p dbt_load_file(const str *tbn, const str *dbn)
if(!fin)
return NULL;

buf = pkg_malloc(buffer_size);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ovidiusas , wouldn't be worthy to have this buf as static var in the function, so we can allocate it upon first usage and keep re-use it for the whole runtime? Instead of keep allocating and freeing it after each load....

Copy link
Member Author

@ovidiusas ovidiusas Apr 1, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could, but this would leave us with unused memory. The buffer is used only during init and during a db reload.
If we make the buffer static, then the buffer will be replicated into all running procs and wasted there.
Before, the buffer was part of the stack and now it will be part of the pkg memory.
We have max 2 pkg mem allocations per table/file (one for version and one for the table itself).
I don't think that we will gain to much (in terms of speed) by making it static. I would prefer to avoid wasting memory.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

agreed, point taken

@ovidiusas
Copy link
Member Author

@bogdan-iancu Any updates here?

@bogdan-iancu
Copy link
Member

@ovidiusas , could you remove the tracer stuff from here and keep only the db_text part?

@ovidiusas
Copy link
Member Author

Done!

@bogdan-iancu bogdan-iancu merged commit 9089106 into OpenSIPS:master Apr 11, 2024
88 checks passed
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