posix: capture readdir d_ino/d_type at scandir enumeration for DirEntry (#68) - #1107
Conversation
binary_int_overflow_local_resume, exc_bridge_entry_guard_not_removed, and list_append_write_barrier_gc across dynasm/cranelift/wasm: bridges_compiled drops by one and guard_failures decreases after the trace-time unary int specialization removes deopts (loops_compiled and loops_aborted unchanged, internal_compile_panics=0). Adds the fbw_blackhole_adopted_multi_frame, fbw_blackhole_adopted_single_frame, and fbw_store_journal_rollback_failed counters (all 0). Values are identical across the three backends. Assisted-by: Claude
… model dir_entry_stat caches and returns the built stat_result object (`entry.stat() is entry.stat()`), which is `posixmodule.c DirEntry_get_stat`, not `interp_scandir.py descr_stat` (which caches raw stat data and rebuilds a fresh result per call). The doc comment cited the latter; correct it and note the object-caching is the 3.14 behavior. Assisted-by: Claude
…tion W_DirEntry gains a native enum_ino field. The name-based scandir path captures the readdir inode (DirEntryExt::ino(), never a stat) and dir_entry_inode returns it directly, the way descr_inode returns self.inode; it is -1 when unavailable (the scandir(fd) path and non-unix hosts), which falls back to the existing stat. A primitive field is GC-safe — the pyre_class macro emits PTR_OFFSETS only for PyObjectRef fields. Assisted-by: Claude
Capture the d_type and d_ino readdir reports at scandir enumeration into W_DirEntry.enum_type / enum_ino. The name path enumerates through opendir/readdir instead of std::fs::read_dir so it reads the dirent directly, and the descriptor path captures the same fields through fd_readdir. is_dir/is_file/is_symlink answer from enum_type unless it is DT_UNKNOWN or a followed symlink, and inode() answers from enum_ino on the descriptor path too, both without a stat. A known DT_DIR/DT_REG therefore answers is_dir()/is_file() from the cached type even after the entry is removed, as descr_is_dir/is_file do. The wasm, sandbox, and non-host_env paths keep the std fallback with enum_type DT_UNKNOWN and stat. Extract readdir_collect and fd_readdir out of fdlistdir, and add join_dir_name for the enumerated full path. Assisted-by: Claude
|
Warning Review limit reached
Next review available in: 41 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
WalkthroughThe PR synchronizes GC test registry walks, adds native Windows timestamp handling, preserves POSIX directory metadata in ChangesFilesystem behavior
GC registry walk synchronization
JIT statistics baselines
JIT attribute specialization
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant PythonCall
participant POSIXInterpreter
participant WindowsAPI
participant FileSystem
PythonCall->>POSIXInterpreter: call utime(path, times)
POSIXInterpreter->>WindowsAPI: CreateFileW(path)
WindowsAPI->>FileSystem: open file or directory handle
POSIXInterpreter->>WindowsAPI: SetFileTime(FILETIME values)
WindowsAPI->>FileSystem: update timestamps
POSIXInterpreter-->>PythonCall: return or report path error
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
🤖 Codex parity reviewStatic analysis of this diff vs the local RPython/PyPy sources (commit 0a711dc). Files in the reviewed diff1. Regressions to PyPy parity introduced by this patch
2. Other mismatches introduced by this patchNone. 3. Pre-existing mismatches (already present before this patch)
4. Structural adaptations
|
…hold `try_walker_specialize_load_type_name_attr` takes the metaclass from `baseobjspace::type_name_obj_fast_path`, which asks `typedef::type`. That falls back to `gettypefor(ob_type)` when the receiver's `w_class` slot is null (typedef.rs:234-243; the objects it covers sit in RODATA, so writing the slot would SIGBUS). The fold then guards the metaclass by reading the raw `w_class` field, so for such a receiver it emits `guard_value(NULL, type)` — a guard that fails on every execution and that nothing can discharge, because no later write fills the slot. One class reaches it. The `getset_descriptor` type object is built lazily from inside the init loop (typedef.rs:9501) as a builder for the descriptors other typedefs install, so it never enters the registry the post-loop sweep walks to stamp `w_class = type` (typedef.rs:1546-1553). `typedef::type` still answers `type` through the fallback, so `type(x)` and `x.__class__` are correct and the null slot is invisible from Python. `synth/pypy_type_surface` reads `type(type.__dict__[name]).__name__`, so it paid one guard failure per iteration on every backend: dynasm, cranelift and wasm all reported `bridges_compiled 5 -> 102, guard_failures 1011 -> 20497` against the figures #999 `dad2a722907` recorded — 19486 excess failures over 20000 iterations, and 19486/200 = 97 excess bridges, one per `trace_eagerness` bucket. #1097 `d51ea7f32a0`, which added the fold, merged 83 seconds before #999, and the recorded figures are the ones this decline reproduces. Localized by taking the fixture apart: `check_descriptor_kinds` alone carries all 97 bridges; the receiver's own value is irrelevant (a loop-invariant object reads the same), the `FOR_ITER` is irrelevant (removing it reads the same), and `int`, `str`, `list`, `NoneType`, `object`, `type`, `method_descriptor`, `wrapper_descriptor`, `builtin_function_or_method` and a user class are all clean. `getattr(C, "__name__")` — same semantics, but the fold declines because the site's name is not in the code's name table — is clean too. check.py dynasm 406/406, cranelift 406/406, wasm 402/402, all green. `synth/pypy_type_surface` reads its recorded 11/5/0/1011 exactly and `synth/type_name_attr_fold` still reads its own 5/0/1/4, so the fold keeps applying wherever its guard holds. The null slot itself is left alone. Stamping `w_class` on the `getset_descriptor` type object would restore the invariant the sweep's comment states and let the fold apply there as well, but it moves recorded baselines and belongs in its own change. Assisted-by: Claude (cherry picked from commit 95ec7f2)
walk_all_gc_tables is reachable from any collector test's collection through the globally-registered gc_table_extra_root_walker once any table has existed. Under the parallel test harness a collector test's collection can upgrade a Weak the dropping registry test expects dead, transiently resurrecting the table and failing "freed table must not be walked". Add a test-only RwLock: every walk takes the read side, a registry test takes the write side across its whole drop/observe window. Split the walk into a locked outer and an unlocked inner so a registry test holding the write side observes the registry without re-entering the lock. Compiled out in production, where the STW collector already prevents a concurrent drop. Assisted-by: Claude
…nd-trip The Windows utime path carried its timestamps as an epoch-based unsigned Duration, so u64::try_from turned away every time before 1970. A FILETIME counts 100-ns intervals from 1601-01-01, which holds a pre-epoch time as an ordinary positive count, so convert the seconds/nanoseconds to a FILETIME and write them with CreateFileW + SetFileTime, the write rposix.win32_utime makes. Failures go through fs_err_with_filename so the OSError carries .winerror, as the module's other Windows path calls do. Enable the Win32_Security feature CreateFileW's signature needs. Assisted-by: Claude
…f_truncate on Windows pathconf and pathconf_names are POSIX-only; Windows CPython has neither, so the section raised AttributeError there under the oracle. Guard it with sys.platform, matching the file's existing utime/truncate Windows guards, and refresh the block comment now that SetFileTime writes pre-epoch times. Assisted-by: Claude
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
https://github.com/youknowone/pyre/blob/e88f41989e0698d452b00e72ac77085ea2093fbd/pyre-interpreter/src/module/posix/interp_posix.rs#L2738
On Unix targets whose struct dirent has no d_type member, this all(unix) function unconditionally accesses a nonexistent field, so pyre-interpreter no longer builds there. The upstream port explicitly handles this case in rpython/rlib/rposix_scandir.py:49-52 by checking HAVE_D_TYPE and returning DT_UNKNOWN; preserve that feature-gated fallback instead of assuming the Linux/macOS layout and hardcoded constants.
AGENTS.md reference: AGENTS.md:L231-L232
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
`pypy_type_surface` reads bridges_compiled=5 and guard_failures=1011 on all three backends, the figures #999 `dad2a722907` recorded. #1097 `d51ea7f32a0` moved them to 102/20497 by folding `Cls.__name__` across a `getset_descriptor` whose `w_class` slot is null, and #1107 `9614f37fb1e` declined that fold again; #1107's own message records the fixture reading 11/5/0/1011 afterwards. #1086 `e5eff816849` sits between the two and wrote the regressed pair in as the baseline, so the restored reading has failed the gate on every runner since. The same commit reverted three wasm guard_failures counters to their pre-#1106 values while keeping #1106 `4555e3d76ba`'s other fields: ca_bridge_multiframe_resume_double_call 2592 -> 2581, wasm_ca_trampoline_decline 601 -> 404, recursion_memo_branch 4704 -> 4724. The guest reads #1106's values both here and on ubuntu-24.04, the only runner that runs wasm, and each fixture's loops_compiled and bridges_compiled already agree with what both observe. Measured with `pyre/check.py --snapshot-diff` per backend and pattern: dynasm 18/18, cranelift 18/18, and wasm 15/15 for each of the three wasm fixtures. Assisted-by: Claude
1 participant
Summary
Makes
os.scandir'sDirEntrycarry the metadatareaddirreports atenumeration, so
inode()/is_dir()/is_file()/is_symlink()answer without astat, matching
interp_scandir.py/ CPython 3.14. Closes the POSIX legs of #68.Also bundles a #66 doc follow-up, a post-rebase jitstats re-record, a
cherry-picked JIT fix for a base regression the rebase surfaced, and three fixes
for CI reds that were pre-existing base issues unrelated to the scandir work
(see "Bundled CI-red fixes").
Commits
f052143): nativeenum_ino: i64onW_DirEntry; the namepath captures the readdir
d_ino, andinode()returns it without a stat(
-1→ the existing stat fallback).78dca77): nativeenum_type: i32(thed_typebyte; default0=DT_UNKNOWN). The name path enumerates throughlibc::opendir/readdirso it reads the dirent directly; the descriptor pathcaptures the same fields.
is_dir/is_file/is_symlinkanswer fromenum_type, falling back to a stat only forDT_UNKNOWNor a followed symlink.defb136): correct theDirEntry.statcaching doc.78ca18c): re-record three drifted synth baselines(benign:
bridges_compiled−1,guard_failures↓, newfbw_*= 0).8b01d57, cherry-picked-xfrom jit(wasm): drop the wasm32 arm of the self-recursive root-bridge inline #1106): declines theCls.__name__fold when its metaclass guard cannot hold — fixes thepypy_type_surfacebase regression the rebase replayed. De-dups on rebase oncejit(wasm): drop the wasm32 arm of the self-recursive root-bridge inline #1106 lands.
Bundled CI-red fixes
After the type-name fold fix went green locally, PR CI showed two reds; both were
pre-existing base issues, not caused by the scandir work (adjudicated with the
Linux-CI free-control and against the base run at
d936eb4be42). Fixing thepypy_type_surfacegate also unmasked a Windows parity red thatcheck.pyhadbeen bailing before (gate failed → parity tests never ran).
d6c2df5,cargo test (ubuntu)):walk_all_gc_tablesis reachable from any collector test's collection via the globally-registered
gc_table_extra_root_walker. Under the parallel harness a collection canupgrade a
Weaka dropping registry test expects dead, transientlyresurrecting the table ("freed table must not be walked"). Fix: a
#[cfg(test)]RwLock(walks read, registry tests write), compiled out in production.Empirical A/B: pre-fix 37–80 resurrections / 20000 iters → post-fix 0/0; full
suite 20/20 clean.
os.utimepre-epoch (6a7168c,check.py (windows)): the Windowspath carried timestamps as an epoch-based unsigned
Duration, refusing everypre-1970 time. Rewritten to
CreateFileW+SetFileTimethrough aFILETIME(100-ns ticks from 1601), the write
rposix.win32_utimemakes; failures gothrough
fs_err_with_filenamesoOSError.winerroris set. Enables theWin32_SecurityfeatureCreateFileW's signature needs (caught by an isolatedx86_64-pc-windows-msvccross-check).e88f419):os.pathconf_namesis POSIX-only (WindowsCPython lacks it), so guarded that section of
os_utime_pathconf_truncate.pywith
sys.platform, matching the file's existing utime/truncate guards.Verification (macOS, python3.14 oracle)
types, cached-after-removal, follow/nofollow, broken symlink, the
scandir(fd)path, GC stress, and
os.scandir("a\0b")→ValueError.os.walk,glob,pathlib.rglob,shutil.copytree/rmtree.pyre/check.py— ALL PASSED, 3/3 backends (dynasm 405/405, cranelift405/405, wasm 401/401), incl. the unix parity test (oracle OK, pyre OK).
utime FFI review CONFIRMED and I fixed an
io_err→fs_err(winerror) bug.Windows CI leg (a full macOS→Windows cross-compile is blocked by libffi-sys;
the FFI surface was cross-checked in isolation).
Remaining #68 scope
Windows
DirEntry.inode()/is_dirshould use the enumeration file index /FILE_ATTRIBUTEflags instead of a freshwin_stat_fields(). Untestable onmacOS; tracked for Windows CI.
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
utimeupdates for pre-epoch timestamps and directory handles.Bug Fixes
scandirbehavior and metadata caching across POSIX and Windows.__name__attribute lookups.Tests