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

Increase the number of work items used by the allocations test #1852

Merged
merged 2 commits into from
Dec 5, 2023

Conversation

kpet
Copy link
Contributor

@kpet kpet commented Nov 29, 2023

The test uses a fixed number of work items to process very large buffers and images. As devices support more and more memory, this leads to an ever-increasing amount of work done in each work item. This results in some implementations and devices hitting timeout issues.

Furthermore, a greater number of work items can provide performance benefits on some devices.

Long term, this test should be redesigned to scale the number of threads dynamically as a function of the max allocation size.

The test uses a fixed number of work items to process very large
buffers and images. As devices support more and more memory, this
leads to an ever-increasing amount of work done in each work item.
This results in some implementations and devices hitting timeout
issues.

Furthermore, a greater number of work items can provide performance
benefits on some devices.

Long term, this test should be redesigned to scale the number
of threads dynamically as a function of the max allocation size.

Signed-off-by: Kevin Petit <kevin.petit@arm.com>
@kpet
Copy link
Contributor Author

kpet commented Nov 29, 2023

The issue this PR helps with has been seen on the following implementations:

  • Arm Mali driver
  • clvk on top of NVidia's proprietary Vulkan driver.

Copy link
Contributor

@bashbaug bashbaug left a comment

Choose a reason for hiding this comment

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

I'm OK with this, but FWIW it doesn't seem to have much of an affect on the Intel devices I tested, positively or negatively.

@kpet
Copy link
Contributor Author

kpet commented Dec 5, 2023

Merging as agreed in the 2023/12/05 teleconference.

@kpet kpet merged commit 0fa6f23 into KhronosGroup:main Dec 5, 2023
7 checks passed
@kpet kpet deleted the allocations-use-more-threads branch December 5, 2023 17:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants