Skip to content

TSRM should set a smarter value for expected_threads #10854

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.

Sign up for GitHub

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

Jump to bottom
Closed
nielsdos opened this issue Mar 14, 2023 · 0 comments · Fixed by #10867
Closed

TSRM should set a smarter value for expected_threads #10854

nielsdos opened this issue Mar 14, 2023 · 0 comments · Fixed by #10867
Labels
Feature Status: Needs Triage

Comments

@nielsdos
Copy link
Member

Description

The tsrm_startup() function is currently always called with expected_threads = 1. This means that the hashtable used in the TSRM will only contain a single bucket, and all thread resources will therefore be in the same linked list. So it's not really a hashtable atm, even though it's supposed to be.
We could improve performance in principle by choosing a more appropriate value for expected_threads, by somehow letting the SAPI give an estimation based on its configuration.

I noticed this while working on #10737.
nielsdos added a commit to nielsdos/php-src that referenced this issue Mar 16, 2023
@nielsdos
Implement phpGH-10854: TSRM should set a smarter value for expected_t…
93c3e6a 
…hreads

The tsrm_startup() function is currently always called with expected_threads = 1.
This means that the hashtable used in the TSRM will only contain a single bucket,
and all thread resources will therefore be in the same linked list.
So it's not really a hashtable right now, even though it's supposed to be.

This patch adds a function tsrm_startup_ex() which takes the expected
thread count as an argument. It also keeps the tsrm_startup() function
so there are no BC breaks.

In the Apache SAPI we query how many threads we have, and pass that to
the tsrm_startup_ex() function.
@nielsdos nielsdos mentioned this issue Mar 16, 2023
Merged
nielsdos added a commit that referenced this issue Mar 17, 2023
@nielsdos
Implement GH-10854: TSRM should set a smarter value for expected_thre…
4da0da7 
…ads (#10867)

The tsrm_startup() function is currently always called with expected_threads = 1.
This means that the hashtable used in the TSRM will only contain a single bucket,
and all thread resources will therefore be in the same linked list.
So it's not really a hashtable right now, even though it's supposed to be.

This patch adds a function tsrm_startup_ex() which takes the expected
thread count as an argument. It also keeps the tsrm_startup() function
so there are no BC breaks.

In the Apache SAPI we query how many threads we have, and pass that to
the tsrm_startup_ex() function.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Feature Status: Needs Triage
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Implement GH-10854: TSRM should set a smarter value for expected_threads nielsdos/php-src
1 participant
@nielsdos