Skip to content

Sync upstream/main: compile uncached where numba can write no cache (f8a6fe6) - #116

Merged
nelson2005 merged 9 commits into
mainfrom
sync/upstream-2026-10-03
Oct 3, 2026
Merged

nelson2005 merged 9 commits into
mainfrom
sync/upstream-2026-10-03

Conversation

@nelson2005

Copy link
Copy Markdown
Owner

Merges upstream's Goykhman@f8a6fe6 into fork main. It carries the seven commits of Goykhman#42: where numba can write no cache (an archive import, a read-only install), numbox compiles uncached after one warning naming the remedy instead of dying at import.

Clean merge, no conflicts. Outside the fork-only files the merged tree is upstream's, and it is identical to the head of #115.

nelson2005 and others added 9 commits October 1, 2026 15:28
…t died at its first decorated function

numba sets a cached function up when it is decorated and raises there when no cache location can be written: an import from an .egg, .whl or .pyz archive, or a read-only install whose user cache directory cannot be written either. A .zip is given the user's cache directory from numba 0.61 on without a check that it can be written, and died with OSError at the first save instead. Every module decorates under the one jit_options, so the question is put once, when configurations is imported, and answered for the package: a function whose file is one of the package's is given to CompileResultCacheImpl, the way numba asks at decoration, then its source stamp read and its cache path ensured, the writability check of the first save; nothing is compiled. One module of each directory of the package is asked, since numba's in-tree cache is a __pycache__ beside each source, found from this module's __file__, each real directory once; a module that survives as .pyc alone, or a .pyc member of a .zip that zipimport would run, is asked by the file its code was compiled from, which is what numba looks up. Where any answer is no, jit_options comes back with cache off and one RuntimeWarning names the remedy for the placement: NUMBA_CACHE_DIR for a source file on disk; the user's cache directory made writable, or put at a shorter path, for a .zip or a frozen application, which numba caches there whatever the variable says; the source files on disk, or a .zip holding them, for any other archive or a .pyc without its source; and NUMBOX_JIT_OPTIONS='{"cache": false}' to turn caching off and silence it. An error that is not the cache's is raised as it was. A NUMBOX_JIT_OPTIONS value that is not a JSON object, or whose cache is not true or false, is refused by name; one without a cache key leaves njit to its default, off, but the sqlite callbacks cache under it, so the question is put for those too. Thirty tests place the tree in each archive and each tree where numba can cache nothing, or not everything, and import it in a child; a docs page covers the options, where the cache lands, the fallback and each remedy.
…ut 93 characters overflowed them

numba names its cache files after the anchor's stem and the generated function's qualified name, and both carried the struct's name, so a struct named with about 93 characters died with OSError: File name too long in numba's own files, past the anchor's write. bounded_stem keeps a name of 40 bytes or fewer as it is, so nearly every struct keeps the file names it had, and cuts a longer one to the whole characters within 31 bytes and a digest of the whole; the measure is the name's UTF-8, which is the file system's, so a name of 40 accented or CJK characters is bounded too. make_structref defines the generated class, the field getters, the method thunks and the make_ and ol_ functions under bounded names, a long field's getter handed to the field's name and kept clear of the other fields' and the methods', and the class takes the struct's full __name__ and __qualname__ back once its body is compiled. The longest file numba writes for any struct, a method thunk's, stays under 230 bytes, with numba's temporary name at the write 21 bytes longer. Six tests build structs named with 40 to 300 ASCII, accented and CJK characters, each with a field of the struct's length, a field named with that one's bounded name, a method of the bounded length and one of 200 characters, cache them and load them again in a second process.
…the generated code uncached where there is none; it died at the write or at numba's set-up

The code numbox generates at run time, make_structref's, compile_kernel's, the work builder's derives and the sqlite registrations', is anchored to a file under NUMBA_CACHE_DIR or the user's cache directory, which can be unwritable where the package's own files cache beside their sources; make_graph's kernel is anchored to the builder's own file and cached beside it. The anchor is written whenever it can be, as before, since numba quotes the source from it in its messages, and each anchor now puts the package's question for its own file when caching (_anchored_or_uncached), so the code it names compiles without a cache after a warning of the same shape where the answer is no, instead of dying at the write or at numba's set-up; the builder's derive falls back without a warning, as it did, and an uncached kernel has no anchor, as before. make_structref, compile_kernel and the builder take jit options of the caller's, which NUMBOX_JIT_OPTIONS does not reach, and compile_kernel's cache argument overrides those too, so the warning's silence is cache off in the options the code was given, the argument where it takes one, or the variable where the options are the package's; make_graph puts the package's question for the builder's file under those options, for its kernel alone, since that kernel took a caller's cache to numba past the package's answer and died from an archive. A path too long for the file system is one such failure, and the warning says so, offering NUMBA_CACHE_DIR at a shorter path, which moves the anchor out of a long user cache directory too, the names being bounded; a directory within about 230 bytes of the path limit passes the check and overflows at numba's first save instead, which the docs say. Twenty-four tests run each anchor writer with its cache directory unwritable, a file, or a component too long for a path, warm in a directory that stopped being writable, and under a NUMBA_CACHE_DIR or a home too deep for the anchor's name; make_graph under a caller's cache from an archive; the warning for options the caller gave, compile_kernel's cache argument among them; and a typing error quoting its line with caching off.
…mit whatever tmp_path's length; from a base directory of 122 bytes it ended past the limit and its mkdir died before the test ran
…es; a location within their length of the path limit passed numba's temporary file and the import died at the first save

numba's writability check makes a temporary file, one without a name on Linux, and the files it saves are named after the module's stem, the function's qualified name, a line number, the interpreter tag and an index number, under a 21-byte temporary name: 105 bytes for the longest-named function of the package, 117 for the builder's generated kernel. A NUMBA_CACHE_DIR whose location for a module came within that of the path limit passed the probe, as it passes numba, and the import died at numba's first save with File name too long. The probe makes and removes a file of 128 bytes in each location it asks about, the package's bound on numba's names, which a test holds every function of the package under, and the warning's remedy for a source on disk names the length: a shorter NUMBA_CACHE_DIR, or none, since each location numba picks for a source on disk but the one beside it appends the source's directory path, else the package at a shorter path. The anchors keep numba's own check, their names being the struct's; a NUMBA_CACHE_DIR deep enough to overflow an anchor's name is too deep for the package's files first, so the package answers, which the anchors' path-limit test pins now, its home case as before.
…mbox builds its other generated functions, rather than replacing a function's co_filename; numba's locators read the same file, cache path, source stamp and errors either way, numba 0.60 to 0.67
…bility check numba's .zip locator is missing
…cache-can-be-written

Compile uncached where numba can write no cache for numbox, with one warning naming the remedy
Copilot AI balanced review requested due to automatic review settings October 3, 2026 20:44
@github-actions

github-actions Bot commented Oct 3, 2026

Copy link
Copy Markdown

MiniMax Code Review

The user wants me to review a pull request. Let me analyze the changes carefully.

This is a significant PR that adds functionality for handling numba caching in various edge cases. Let me understand what's being done:

  1. Documentation changes: Adding docs for numbox.core.configurations module

  2. Core logic in numbox/core/configurations.py:

    • get_jit_options() now validates the JSON format and the cache option
    • New functions for cache probing: check_cache_location(), is_a_cache_error(), _module_files(), _sourceless_compiled_from(), _archived_module_files(), _compiled_from(), cache_remedy(), uncached_where_no_cache_can_be_written()
    • The module now checks at import time whether numba can cache for numbox's files
  3. Changes to numbox/utils/preprocessing.py:

    • Added bounded_stem() function to limit file name lengths
    • Changed _anchor_path() to use bounded stems
    • Added new functions: _anchor_or_error(), _anchored_or_uncached(), _cached_at_or_uncached()
  4. Changes to code generation:

    • numbox/core/variable/compile_kernel.py: Uses _anchored_or_uncached()
    • numbox/core/work/builder.py: Uses new anchor functions
    • numbox/core/bindings/sqlite/tvf.py and udf_helpers.py: Uses _anchored_or_uncached()
    • numbox/utils/highlevel.py: Major changes for bounded names in struct generation
  5. Tests: Added comprehensive tests for various scenarios

Let me provide a code review:

Strengths:

  1. Well-documented with extensive comments

  2. Comprehensive test coverage for edge cases

  3. Good error handling with specific error messages

  4. Graceful degradation when caching isn't possible

Potential Issues:

  1. In configurations.py, the check_cache_location function uses exec(compile(...)) to create a probe function. This is intentional for testing cache writability, but security-conscious reviewers might flag it. The code passes a hardcoded string so it's safe in this context.

  2. In _module_files(), the function recursively walks through package directories to find all module files, which could be expensive for large packages. The use of os.walk() with followlinks=True might introduce symlink traversal risks, though the code does check for cycles.

  3. The _SILENCE constant contains a hardcoded string that appears designed for warning messages, but it's unclear where it's used or if it's meant to be user-configurable.

  4. Looking at bounded_stem(), there's a potential encoding issue: when truncating to _STEM_MAX - 9 bytes and decoding as UTF-8 with errors="ignore", this could produce invalid UTF-8 sequences that get re-encoded, potentially creating unexpected behavior.

The implementation appears sound overall—the test suite seems thorough and the approach is reasonable.

Code Review: Numba Caching Support PR

This is a substantial PR that adds robust handling for numba caching in various edge cases (read-only installs, zip archives, path length limits, etc.). Here's my analysis:

Summary

The PR adds the ability for numbox to gracefully degrade when numba cannot write a cache, instead of crashing at the first decorated function. It also bounds long names to prevent filesystem limits.

Strengths

  1. Comprehensive test coverage: The new test file test_no_cache_location.py has 1100+ lines covering many edge cases
  2. Good error messages: Warnings clearly explain the problem and remedies
  3. Defensive coding: The implementation errs toward uncached (safer) rather than risking crashes

Minor Issues/Suggestions

1. Potential encoding issue in bounded_stem() (preprocessing.py:59)

head = encoded[:_STEM_MAX - 9].decode("utf-8", errors="ignore")

This could produce invalid UTF-8 sequences when truncating mid-character, then re-encoding. Consider using errors="replace" or validating the result.

2. Security: Use of exec (configurations.py:80)

exec(compile("def _cache_probe():\n    pass\n", os.fspath(py_file), "exec"), namespace)

This is intentional and the source is controlled, but future maintainers should be aware. A comment explaining why this is necessary would help.

3. Missing error handling in _archived_module_files() (configurations.py:186)

with zipfile.ZipFile(zip_path) as archive:

If the zip file is corrupted, this will raise an exception. Should this be caught and handled gracefully like other archive issues?

4. Test organization

The test file test_no_cache_location.py is very long (1100+ lines). Consider splitting into multiple files by scenario type for maintainability.

Questions for the Authors

  1. What's the rationale for the 40-byte stem limit? Was this chosen empirically, or is there a specific calculation?

  2. Why does make_graph use _cached_at_or_uncached() for the kernel but not for the derives? The docstring mentions this is intentional—could you clarify?

  3. Is there a reason _SILENCE is a module-level constant rather than a parameter? It seems like it could vary by caller.

Verdict

The code looks well-engineered and the tests are thorough. The changes are backward-compatible (caching still defaults to on). The main concerns are minor:

  • ✅ Logic is sound
  • ✅ Error handling is comprehensive
  • ✅ Documentation is excellent
  • ⚠️ Consider the encoding edge case in bounded_stem()
  • ⚠️ Consider adding a comment about the exec usage for future maintainers

Recommendation: Approve with optional suggestions above.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot review overview

🟢 Approval recommended

The synchronized upstream change is coherent, thoroughly tested, documented, and introduces no unresolved correctness issues.

Review effort: Balanced
Findings: None

What changed in this PR

Synchronizes the fork with upstream’s graceful fallback when Numba caches are unavailable.

Changes:

  • Detects unusable cache locations and compiles uncached with actionable warnings.
  • Safely bounds generated cache filenames.
  • Adds extensive tests and cache-behavior documentation.
File Description
numbox/​core/​configurations.py Adds cache probing, fallback, and option validation.
numbox/​utils/​preprocessing.py Adds anchor fallback and bounded names.
numbox/​utils/​highlevel.py Applies safe naming and fallback to structrefs.
numbox/​core/​work/​builder.py Protects generated graph and derive caching.
numbox/​core/​variable/​compile_kernel.py Uses shared cache fallback.
numbox/​core/​bindings/​sqlite/​udf_helpers.py Protects generated UDF callbacks.
numbox/​core/​bindings/​sqlite/​tvf.py Protects generated TVF callbacks.
test/​core/​test_no_cache_location.py Covers cache-location and filename edge cases.
test/​core/​test_jit_options.py Tests environment-option validation.
test/​core/​test_compile_kernel.py Updates fallback warning assertion.
docs/​numbox.core.configurations.rst Documents cache configuration and remedies.
docs/​numbox.core.variable.rst Documents kernel fallback behavior.
docs/​numbox.utils.rst Documents anchors and bounded names.
docs/​modules.rst Adds configuration documentation to the index.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@nelson2005
nelson2005 merged commit 0926d1c into main Oct 3, 2026
37 of 38 checks passed
@nelson2005
nelson2005 deleted the sync/upstream-2026-10-03 branch October 3, 2026 21:09
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.

3 participants