cyrush pushed to LLNL/conduit
changelog
tgamblin pushed to spack/spack
Spec: use short-circuiting, stable comparison
Background
Spec comparison on develop used a somewhat questionable optimization to
get decent spec comparison performance – instead of comparing entire spec
DAGs, it put a hash()
call in _cmp_iter()
and compared specs by their
runtime hashes. This gets us good performance abstract specs, which don’t
have complex dependencies and for which hashing is cheap. But it makes
the order of specs unstable and hard to reproduce.
We really need to do a full, consistent traversal over specs to compare
and to get a stable ordering. Simply taking the hash out and yielding
dependencies recursively (i.e. yielding dep.spec._cmp_iter()
instead
of a hash) goes exponential for concrete specs because it explores all
paths. Traversal tracks visited nodes, but it’s expensive to set up
the data structures for that, and it can slow down comparison of simple
abstract specs. Abstract spec comparison performance is important for
concretization (specifically setup), so we don’t want to do that.
New comparison algorithm
We can have (mostly) the best of both worlds – it’s just a bit more complicated.
This changes Spec comparison to do a proper, stable graph comparison:
-
Spec comparison will now short-circuit whenever possible for concrete specs, when DAG hashes are known to be equal or not equal. This means that concrete spec
==
and!=
comparisons will no longer have to traverse the whole DAG. -
Spec comparison now traverses the graph consistently, comparing nodes and edges in breadth-first order. This means Spec sort order is stable, and it won’t vary arbitrarily from run to run.
-
Traversal can be expensive, so we avoid it for simple specs. Specifically, if a spec has no dependencies, or if its dependencies have no dependencies, we avoid calling
traverse_edges()
by doing some special casing.
The _cmp_iter
method for Spec
now iterates over the DAG and yields nodes
in BFS order. While it does that, it generates consistent ids for each node,
based on traversal order. It then outputs edges in terms of these ids, along with
their depflags and virtuals, so that all parts of the Spec DAG are included.
The resulting total ordering of specs keys on node attributes first, then
dependency nodes, then any edge differences between graphs.
Optimized cases skip the id generation and traversal, since we know the order and therefore the ids in advance.
Performance ramifications
Abstract specs
This seems to add around 7-8% overhead to concretization setup time. It’s worth the cost, because this enables concretization caching (as input to concretization was previously not stable) and setup will eventually be parallelized, at which point it will no longer be a bottleneck for solving. Together those two optimizations will cut well over 50% of the time (likely closer to 90+%) off of most solves.
Concrete specs
Comparison for concrete specs is faster than before, sometimes way faster because comparison is now guaranteed to be linear time w.r.t. DAG size. Times for comparing concrete Specs:
def compare(json):
a = spack.spec.Spec(json)
b = spack.spec.Spec(json)
print(a == b)
print(timeit.timeit(lambda: a == b, number=1))
compare("./py-black.json")
compare("./hdf5.json")
develop
(uses our prior hash optimization):py-black
: 7.013e-05spy-hdf5
: 6.445e-05s
develop
with full traversal and no hash:py-black
: 3.955spy-hdf5
: 0.0122s
- This branch (full traversal, stable, short-circuiting, no hash)
py-black
: 2.208e-06spy-hdf5
: 3.416e-06s
Signed-off-by: Todd Gamblin tgamblin@llnl.gov</small>
cyrush pushed to Alpine-DAV/ascent
ba: add cmake_hip_compiler
brugger1 pushed to visit-dav/visit
Update the last test suite pass on poodle.
homeomorfismo pushed to mfem/mfem
Header renaming
victorapm pushed to hypre-space/hypre
Merge branch ‘sycl-warning’ into geos-next
mplegendre pushed to hpc/Spindle
Merge pull request #69 from mplegendre/bugfix/procmaps_reading
Hide spindle paths found in /proc/pid/maps</small>
JustinPrivitera pushed to LLNL/conduit
mpi mesh examples module
BradWhitlock pushed to LLNL/axom
Updated data directory version.
nicolemarsaglia pushed to Alpine-DAV/ascent
Merge branch ‘task/2025_03_warpx_unit_test’ of https://github.com/Alpine-DAV/ascent into task/2025_03_warpx_unit_test
emily-howell pushed to Alpine-DAV/ascent
Only run if ascent was built with rendering support
adrienbernede pushed to LLNL/Umpire
Finish enforcing rocm suite
adrienbernede pushed to LLNL/CHAI
Update comment
tzanio pushed to GLVis/glvis
Merge pull request #327 from GLVis/2d3v-vis
Visualization of 2D grid functions with 3 vector dimensions</small>
adrienbernede pushed to LLNL/CARE
Update uberenv config
davidbeckingsale pushed to LLNL/Umpire
Add deviceconst test on Tioga
github-actions[bot] pushed to LLNL/Umpire
Apply style updates
mergify[bot] pushed to flux-framework/flux-sched
Merge pull request #1333 from jameshcorbett/find-by-name
evaluators: find by name and by property</small>
Merge pull request #264 from vicentebolea/extend-windows-builds
ci,windows: enable more opts</small>
daboehme pushed to LLNL/Caliper
Keep input type when computing averages (#643)
najlkin pushed to GLVis/glvis
Updated CHANGELOG.
Jay-A pushed to GEOS-DEV/LvArray
updated src files to match develop and fixed incorrect LLNL TPL_DIR path