Fix non-strict HIP device lib order #2217
Conversation
|
could you please add a test for this ? thanks |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2217 +/- ##
==========================================
+ Coverage 30.91% 40.82% +9.91%
==========================================
Files 53 55 +2
Lines 20112 20675 +563
Branches 9755 9801 +46
==========================================
+ Hits 6217 8440 +2223
- Misses 7922 8100 +178
+ Partials 5973 4135 -1838 ☔ View full report in Codecov by Sentry. |
Snektron
force-pushed
the
fix-device-libs-order
branch
from
4d87a24 to
c3eb3c5
Compare
|
Im not 100% sure what the best way is to add the tests, considering that fully testing that issue requires deleting and then re-adding a file from the HIP bitcode directory ( |
Snektron
force-pushed
the
fix-device-libs-order
branch
3 times, most recently
from
e0a58c1 to
48d6ed7
Compare
|
bump |
|
still need tests, sorry :) |
Snektron
force-pushed
the
fix-device-libs-order
branch
4 times, most recently
from
3d28979 to
b2355f7
Compare
|
Seems that fuse doesn't work in github actions. I guess the only other approach swapping out calls to |
|
Hmm, it might be possible to LD_PRELOAD a library which returns random results from readdir... |
Snektron
force-pushed
the
fix-device-libs-order
branch
12 times, most recently
from
84c025c to
c7c9136
Compare
|
@sylvestre What do you think about this approach? It seems to be working fine in our fork's CI, though I feel like theres some confirmation missing that the shim is actually running. One details is that I've had to set LD_PRELOAD separately on each run, it seems that using |
| @@ -0,0 +1,196 @@ | |||
| use ctor::ctor; | |||
There was a problem hiding this comment.
please add the license header
This is a simple libc shim that randomizes the results of readdir(). For some functionality, cache hashes are dependent on the order that values are returned from fs::read_dir(), which calls lib readdir(). This library can be inserted into the target application's runtime using LD_PRELOAD=target/debug/librandmize_readdir.so, after which the results of readdir() will return in random order. It is only intended to be used while testing sccache.
Snektron
force-pushed
the
fix-device-libs-order
branch
from
c7c9136 to
7c11cce
Compare
In #2045 initial support for HIP was added. While trying it out, I noticed that different runs across different machines did not yield an expected cache hit. After some investigation, it turns out that the list of bitcode device libraries is not sorted after discovering them from the file system, and this resulted in a different order across those machines. I've added a fix that simply sorts those libraries after discovering them, that seems to be similar to how its handled elsewhere.