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

Improve lock contention in render_list etc #335

Merged
merged 3 commits into from
Sep 27, 2023

Conversation

tomhughes
Copy link
Member

This attempts to minimise the time that the lock is help when taking a job from the queue and scales the queue size with the number of threads to try and ensure it stays full enough to keep all threads busy.

cmd->z = e->z;
cmd->x = e->x;
cmd->y = e->y;
strncpy(cmd->xmlname, e->mapname, XMLCONFIG_MAX - 1);

Check notice

Code scanning / Flawfinder

Easily used incorrectly; doesn't always \0-terminate or check for invalid pointers [MS-banned] (CWE-120). Note

buffer/strncpy:Easily used incorrectly; doesn't always \0-terminate or check for invalid pointers [MS-banned] (CWE-120).
Copy link
Contributor

Choose a reason for hiding this comment

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

Since this existing code with no changes except indentation and position, I don't think CWE-120 is a blocker.

Copy link
Contributor

@pnorman pnorman left a comment

Choose a reason for hiding this comment

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

My estimate based on gdb is that 5% of the threads on an 80 core server are waiting for a lock, and this could help reduce that. It's not game changing, but a worthwhile improvement.

cmd->z = e->z;
cmd->x = e->x;
cmd->y = e->y;
strncpy(cmd->xmlname, e->mapname, XMLCONFIG_MAX - 1);
Copy link
Contributor

Choose a reason for hiding this comment

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

Since this existing code with no changes except indentation and position, I don't think CWE-120 is a blocker.

@xamanu xamanu merged commit a8639be into openstreetmap:master Sep 27, 2023
1 check passed
@tomhughes tomhughes deleted the queue-concurrency branch September 27, 2023 07:47
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.

None yet

3 participants