https://pantsbuild.org/ logo
Join Slack
Powered by
I think caching for the JVMProcess doesn't work fo...
# general
m
I think caching for the JVMProcess doesn't work for us, actually any change in the code (or BUILD files) at the moment leads to full compilation of the tree. Any good inputs on how to debug this?
w
Looking at the 
-ldebug
logs for two successive runs with 
--no-pantsd
where the second one misses should show you the fingerprints for the processes that are being invoked. If you're comfortable including snippets here (or DMing), I can take a look.
Oh. But you mentioned actual code changes... So probably you'll want to look at a particular upstream file and then use 
./pants paths --from=$file1 --to=$file2
...to see why a file you weren't expecting to have been compiled (file1) ended up changing when another file has changed (file2)
m
cache should be preserved between pantsd instances, right? It happens both with upstream version and pants_from_source, pants_from_source should also be cached right?
w
It happens both with upstream version and pants_from_source, pants_from_source should also be cached right?
yes, both should be cached, although different versions of Pants will have different cache keys
if there are changes to files, and so you are expecting something to be recompiled, you’ll want to try out 
paths
as mentioned above… it will tell you why a dependee needs to be recompiled
or look at 
./pants dependees $changed-file
to see what depends on a file
m
cool, that is helpful
I will try that
w
looking at the debug level logs will mostly only be useful if two runs in a row with no changes and 
--no-pantsd
actually recompile things on the second run
m
thanks, I was able to reproduce without any file change. 
ScalaPlugins
produced unordered arguments.
Will send a pr to fix today along with the difference I found in the log
w
yikes! thanks for the fix.
this relates to https://github.com/pantsbuild/pants/issues/14195#issuecomment-1114087237 … there are definitely more static checks that we can/should do here. will be curious to see whether it was “iterating over a `set`” in this case
m
I will make a draft PR, still checking if the fix cover all the cases.
Gist of two runs, exact same code: https://gist.github.com/somdoron/f6c824923db65bda94a28ebdf6a13457 You can notice the order of the scala-plugins. I've compared using https://www.diffchecker.com/diff
I think Classpath and ClasspathEntry could benefit from sorting and removing duplicates.
w
while it would make our lives easier, folks generally have an expectation that the tool will preserve the order of explicit dependencies
for example: two sets of 
resources
that should be loaded in a particular order… or a classpath entry that you want to take precedence over another
m
I agree, but Classpath.args for example might include the same dependency multiple times. However, I don't think we want to miss the cache if the args changed but still have same dependencies.
w
@mysterious-waiter-14207: 
Snapshot
and 
Digest
sort, and merging them dedupes identical entries (and explode for same-name-different-content), so it doesn’t actually result in more copies of anything
so when executing a process, the impact of the 
ClasspathEntry
ordering and duplicates is really just to affect the arguments for the processes
m
I agree re the digest, but Classpath.args returns iterable of strings, which is an input the scalac.
w
yea. in that case you could maybe preserve order and remove dupes
m
Right, but the arguments for the processes affect the cache hit.
w
they need to, yea. because a different classpath ordering can have a different result.
m
didn't know that
interesting
w
but AFAIK, 
A:B:A
is equivalent to 
A:B
, since the first matching class is the one that is used.
m
cool, so only removing duplicates.
w
yea.
thanks!
m
so somwhere in the code the order of the filenames of ClasspathEntry is not consistent as well as ScalaPlugins names.
does multiget guarantee ordering?
w
assuming that the plugin is a thirdparty plugin, it seems likely that it is Coursier.
yes, MultiGet guarantees ordering
…blargh. i think i already see it.
oh… hm. no… that’s iterating over a dictionary, which should be deterministic.
m
For what it worth, in the gist above the ScalaPlugins.names are causing the issue, but I have examples of ScalaPlugins.classpath.filenames as well.
w
that could definitely do it, yea.
dict
ordering is preserved, but 
set
is randomized per run. so sets are fine unless they are iterated over. particularly if that ends up being the issue here, i’ll want to follow up to add a lint á la https://github.com/pantsbuild/pants/issues/14195#issuecomment-1114087237
m
cool, I can replace that with OrderedSet and report.
It seems to work now, thanks!
🎉 1
re removing duplicates from Classpath.args, I'm not feeling strongly about it, as it most mostly theoretical at the moment. Let me know if you want me to add it to the PR or to drop it for now.
w
Seems fine to ignore, yea. If you want to mention it in a TODO without a ticket, that's fine too.
6 Views
Open in Slack
PreviousNext
Powered by