From c38fb379a7a91b031b5158f17849406f506c4ef7 Mon Sep 17 00:00:00 2001 From: Hellblazer Date: Sat, 24 Jan 2026 13:49:49 -0800 Subject: [PATCH] Improve documentation and error messages for build options support - Fix misleading recompile() javadoc: clarify that old kernel resources are released immediately - Add thread safety documentation to recompile() with exclusive access requirement - Enhance error message for double-compilation to suggest using recompile() - Add security note to compile(buildOptions) about unsanitized compiler options - Simplify logging format in compileInternal() - Add javadoc to key test methods for clarity Addresses code review feedback from build options feature (PR #5). All documentation improvements, no functional changes. --- .../resource/compute/ComputeKernel.java | 19 ++++++++++++++----- .../resource/compute/opencl/OpenCLKernel.java | 11 ++++------- .../ComputeKernelBuildOptionsTest.java | 13 +++++++++++++ 3 files changed, 31 insertions(+), 12 deletions(-) diff --git a/resource/src/main/java/com/hellblazer/luciferase/resource/compute/ComputeKernel.java b/resource/src/main/java/com/hellblazer/luciferase/resource/compute/ComputeKernel.java index 2a82e42..0c090b7 100644 --- a/resource/src/main/java/com/hellblazer/luciferase/resource/compute/ComputeKernel.java +++ b/resource/src/main/java/com/hellblazer/luciferase/resource/compute/ComputeKernel.java @@ -54,9 +54,14 @@ public interface ComputeKernel extends AutoCloseable { *
  • Architecture-specific tuning: {@code "-D__GCN_REV__=2"}
    • * * + *

    Security Note: Build options are passed directly to the backend compiler without + * sanitization. Ensure options originate from trusted sources only to prevent compiler-based + * denial-of-service or unexpected behavior. + * * @param source Kernel source code (Metal or OpenCL) * @param entryPoint Kernel entry point function name - * @param buildOptions Compiler flags and preprocessor defines (null or empty for defaults) + * @param buildOptions Compiler flags and preprocessor defines (null or empty for defaults). + * Passed directly to the backend compiler without validation. * @throws KernelCompilationException if compilation fails * @see #recompile(String, String, String) */ @@ -69,8 +74,7 @@ default void compile(String source, String entryPoint, String buildOptions) * Recompile an already-compiled kernel with different build options. * *

    Enables runtime GPU auto-tuning by recompiling kernels with different optimization - * parameters without clearing existing kernel state. Useful for performance experiments - * and adaptive optimization strategies. + * parameters. Useful for performance experiments and adaptive optimization strategies. * *

    Recompilation Workflow:

    * 
    {@code
@@ -83,8 +87,13 @@ default void compile(String source, String entryPoint, String buildOptions)
      * kernel.execute(globalSize);  // Compare performance
      * }
    * - *

    Note: Recompilation creates a fresh kernel. The old kernel reference remains - * valid until explicitly closed, allowing multiple kernel variants to coexist. + *

    Note: Recompilation releases the old kernel and program resources, then compiles + * a fresh kernel. The kernel object itself remains valid and usable after recompilation. + * + *

    Thread Safety: During recompilation, {@link #isCompiled()} may briefly return false + * as resources are released and replaced. Concurrent kernel execution from other threads during + * recompilation will fail with IllegalStateException. Callers must ensure exclusive access to + * the kernel object during recompilation. * * @param source Kernel source code (must match original source for consistency) * @param entryPoint Kernel entry point function name diff --git a/resource/src/main/java/com/hellblazer/luciferase/resource/compute/opencl/OpenCLKernel.java b/resource/src/main/java/com/hellblazer/luciferase/resource/compute/opencl/OpenCLKernel.java index 3815134..d7843ea 100644 --- a/resource/src/main/java/com/hellblazer/luciferase/resource/compute/opencl/OpenCLKernel.java +++ b/resource/src/main/java/com/hellblazer/luciferase/resource/compute/opencl/OpenCLKernel.java @@ -89,7 +89,8 @@ public void compile(String source, String entryPoint) throws KernelCompilationEx public void compile(String source, String entryPoint, String buildOptions) throws KernelCompilationException { checkNotClosed(); if (compiled.get()) { - throw new KernelCompilationException("Kernel already compiled"); + throw new KernelCompilationException( + "Kernel already compiled. Use recompile() to recompile with different build options."); } compileInternal(source, entryPoint, buildOptions); @@ -150,12 +151,8 @@ private void compileInternal(String source, String entryPoint, String buildOptio checkCLError(errcode.get(0), "Failed to create OpenCL kernel: " + entryPoint); compiled.set(true); - if (buildOptions != null && !buildOptions.isEmpty()) { - log.debug("Compiled OpenCL kernel: {} (entry point: {}, options: {})", - name, entryPoint, buildOptions); - } else { - log.debug("Compiled OpenCL kernel: {} (entry point: {})", name, entryPoint); - } + log.debug("Compiled OpenCL kernel: {} (entry point: {}, options: {})", + name, entryPoint, buildOptions != null ? buildOptions : "(none)"); } catch (Exception e) { cleanup(); diff --git a/resource/src/test/java/com/hellblazer/luciferase/resource/compute/ComputeKernelBuildOptionsTest.java b/resource/src/test/java/com/hellblazer/luciferase/resource/compute/ComputeKernelBuildOptionsTest.java index 5f8b619..a49bd4f 100644 --- a/resource/src/test/java/com/hellblazer/luciferase/resource/compute/ComputeKernelBuildOptionsTest.java +++ b/resource/src/test/java/com/hellblazer/luciferase/resource/compute/ComputeKernelBuildOptionsTest.java @@ -101,6 +101,10 @@ static void checkOpenCL() { // --- Basic Build Options Tests --- + /** + * Validates that preprocessor defines work with kernel compilation. + * Tests basic -D option passing to the OpenCL compiler. + */ @Test void testCompileWithDefine() { if (!openCLAvailable) return; @@ -152,6 +156,10 @@ void testCompileWithoutFeatureFlag() { // --- Compiler Flags Tests --- + /** + * Validates that OpenCL compiler flags (e.g., -cl-fast-relaxed-math) work correctly. + * These flags enable performance optimizations and are critical for GPU auto-tuning. + */ @Test void testCompileWithCompilerFlags() { if (!openCLAvailable) return; @@ -196,6 +204,11 @@ void testRecompileWithDifferentOptions() { } } + /** + * Validates that recompilation changes the runtime behavior of the kernel. + * First execution multiplies by 2, second by 5, demonstrating that the + * new build options take effect. Critical for GPU auto-tuning workflows. + */ @Test void testRecompileChangesDefineValue() throws Exception { if (!openCLAvailable) return;