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

Disable build caching when using pocl #738

Closed
wants to merge 6 commits into from

Conversation

majosm
Copy link
Contributor

@majosm majosm commented Apr 23, 2024

Fixes #731.

@matthiasdiener
Copy link
Contributor

matthiasdiener commented Apr 23, 2024

I gave this a quick spin (with the example in #731 (comment)) and didn't notice any negative perfomance impact on the second run, as long as the downstream package (i.e., pocl, nvidia cl) has caching enabled.
I tried pocl-cpu, pocl-cuda, nvidia cl, all on Linux (porter). Perhaps we could skip binary caching for all CL implementations?

@inducer
Copy link
Owner

inducer commented Apr 23, 2024

Perhaps we could skip binary caching for all CL implementations?

That's a broad set to generalize over. 🙂 If they all have source -> executable caches, then sure, that'd probably be better. Nvidia has a such a cache, I think. Do you know about AMD and Intel?

pyopencl/__init__.py Outdated Show resolved Hide resolved
if _PYOPENCL_NO_CACHE:
build_descr = "uncached source build (cache disabled by user)"
else:
build_descr = "uncached source build (cache disabled for pocl)"
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
build_descr = "uncached source build (cache disabled for pocl)"
build_descr = "uncached source build (cache disabled because ICD is caching)"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If I add the characterize function suggested below, should I instead change this to say something like "source build via ICD cache"?

Copy link
Owner

Choose a reason for hiding this comment

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

Sure.

no_cache = _PYOPENCL_NO_CACHE
# Turn off caching when using pocl to avoid extra compile from
# get_info(BINARIES). See https://github.com/inducer/pyopencl/issues/731.
from pyopencl.characterize import get_pocl_version
Copy link
Owner

Choose a reason for hiding this comment

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

Maybe add a characterize function on whether there is an effective source -> executable cache? (Specified to return None for "unknown", False for no, and True for yes.)

@majosm
Copy link
Contributor Author

majosm commented Apr 24, 2024

@inducer I'm seeing an intermittent failure in the boxtree CI (here's a failing run and a successful run for the same code). Is this something to be concerned about?

@@ -374,7 +374,7 @@ def _check_arg_size(function_name, num_cl_args, arg_types, devs):


if not cl._PYOPENCL_NO_CACHE:
invoker_cache = WriteOncePersistentDict(
invoker_cache: WriteOncePersistentDict = WriteOncePersistentDict(
Copy link
Owner

Choose a reason for hiding this comment

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

Add key and value types WOPD[..., ...] (also below).

if _PYOPENCL_NO_CACHE:
build_descr = "uncached source build (cache disabled by user)"
else:
build_descr = "source build via ICD cache"
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
build_descr = "source build via ICD cache"
build_descr = "uncached source build, assumed cached by ICD"

@inducer
Copy link
Owner

inducer commented Apr 24, 2024

Is this something to be concerned about?

Kind of, yeah. Can you reproduce it locally?

@inducer
Copy link
Owner

inducer commented Apr 24, 2024

Maybe it has something to do with tests being run in parallel? How good is pocl about locking its cache?

@majosm majosm force-pushed the disable-build-cache-pocl branch 4 times, most recently from d0aabc5 to e8e1c3f Compare April 25, 2024 16:05
@majosm majosm force-pushed the disable-build-cache-pocl branch 6 times, most recently from be06e3f to 5cd5fd9 Compare April 25, 2024 18:38
@matthiasdiener
Copy link
Contributor

matthiasdiener commented May 1, 2024

Perhaps we could skip binary caching for all CL implementations?

That's a broad set to generalize over. 🙂 If they all have source -> executable caches, then sure, that'd probably be better. Nvidia has a such a cache, I think. Do you know about AMD and Intel?

It turns out that AMD rocm does not appear to cache built kernels :-( (tested with rocm 5.7.1 and 6.0.3 on tioga).

@matthiasdiener
Copy link
Contributor

This has been merged as part of #749.

@majosm majosm closed this May 30, 2024
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.

build program times increasing with rank count on Mac when caching is enabled
3 participants