https://pantsbuild.org/ logo
Powered by
m

mysterious-waiter-14207

07/18/2022, 6:42 AM
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

witty-crayon-22786

07/18/2022, 2:58 PM
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

mysterious-waiter-14207

07/18/2022, 5:55 PM
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

witty-crayon-22786

07/18/2022, 5:55 PM
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

mysterious-waiter-14207

07/18/2022, 5:57 PM
cool, that is helpful
I will try that
w

witty-crayon-22786

07/18/2022, 5:58 PM
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

mysterious-waiter-14207

07/19/2022, 9:16 AM
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

witty-crayon-22786

07/19/2022, 6:39 PM
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

mysterious-waiter-14207

07/19/2022, 7:07 PM
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

witty-crayon-22786

07/19/2022, 7:39 PM
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

mysterious-waiter-14207

07/19/2022, 7:42 PM
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

witty-crayon-22786

07/19/2022, 7:45 PM
@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

mysterious-waiter-14207

07/19/2022, 7:46 PM
I agree re the digest, but Classpath.args returns iterable of strings, which is an input the scalac.
w

witty-crayon-22786

07/19/2022, 7:47 PM
yea. in that case you could maybe preserve order and remove dupes
m

mysterious-waiter-14207

07/19/2022, 7:47 PM
Right, but the arguments for the processes affect the cache hit.
w

witty-crayon-22786

07/19/2022, 7:47 PM
they need to, yea. because a different classpath ordering can have a different result.
m

mysterious-waiter-14207

07/19/2022, 7:48 PM
didn't know that
interesting
w

witty-crayon-22786

07/19/2022, 7:48 PM
but AFAIK, 
A:B:A
is equivalent to 
A:B
, since the first matching class is the one that is used.
m

mysterious-waiter-14207

07/19/2022, 7:49 PM
cool, so only removing duplicates.
w

witty-crayon-22786

07/19/2022, 7:50 PM
yea.
thanks!
m

mysterious-waiter-14207

07/19/2022, 7:52 PM
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

witty-crayon-22786

07/19/2022, 7:52 PM
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

mysterious-waiter-14207

07/19/2022, 7:58 PM
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

witty-crayon-22786

07/19/2022, 8:02 PM
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

mysterious-waiter-14207

07/19/2022, 8:04 PM
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

witty-crayon-22786

07/19/2022, 8:18 PM
Seems fine to ignore, yea. If you want to mention it in a TODO without a ticket, that's fine too.
6 Views
Powered by