Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Omnibus of Performance & Testing Improvements #54

Merged
merged 12 commits into from
Nov 21, 2023
Merged

Conversation

aphyr
Copy link
Contributor

@aphyr aphyr commented Nov 16, 2023

I can't keep the separate git branches straight any more, and from our earlier conversation it sounds like you're amenable to a suite of fixes, so here's a single PR.

As a side note, there's some kind of failure in sorted-map-slice and int-map-slice that seems pre-existing... not sure what's up with that exactly.

FAIL in (test-sorted-map-slice) (collection_test.clj:504)
expected: {:result true}
  actual: {:shrunk
           {:total-nodes-visited 26,
            :depth 25,
            :pass? false,
            :result false,
            :result-data nil,
            :time-shrinking-ms 264,
            :smallest [1 1]},
           :failed-after-ms 8,
           :num-tests 1,
           :seed 1700069068231,
           :fail [6822 4794],
           :result false,
           :result-data nil,
           :failing-size 0,
           :pass? false,
           :test-var "test-sorted-map-slice"}
FAIL in (test-int-map-slice) (collection_test.clj:480)
expected: {:result true}
  actual: {:shrunk
           {:total-nodes-visited 26,
            :depth 25,
            :pass? false,
            :result false,
            :result-data nil,
            :time-shrinking-ms 252,
            :smallest [1 1]},
           :failed-after-ms 19,
           :num-tests 1,
           :seed 1700068598677,
           :fail [4682 4819],
           :result false,
           :result-data nil,
           :failing-size 0,
           :pass? false,
           :test-var "test-int-map-slice"}

aphyr added 12 commits November 14, 2023 13:58
The old HTTP repo was inaccessible--it's got an HTTPS URL now.
JDK17 locked down some of the reflection voodoo the test suite wants to
do. We have to add --add-opens to unlock java.base/java.lang and
java.util.
The normal test suite takes forever to run; this makes it easier to
quickly iterate on a small test.
We burn a ton of time in Elle asking for edges in graphs which don't
have them, and each of those requires constructing an expensive
exception. This adds an extra arity to edge, similar to Map.get(), which
returns notFound rather than throwing if an edge does not exist.
We burn a ton of time in Elle asking for edges in graphs which don't
have them, and each of those requires constructing an expensive
exception. This adds an extra arity to edge, similar to Map.get(), which
returns notFound rather than throwing if an edge does not exist.
The normal test suite takes forever to run; this makes it easier to
quickly iterate on a small test.
The SCC routine called .vertices() twice on its input graph, which for
lazily realized graphs could actually take the majority of our CPU time.
This adds a variable and calls .vertices() only once.
Who's got two thumbs and mistook a slow-running test case for successful
completion? This guy.
Map.intersection constructed a new Map without any equality functions,
which meant that equality semantics (and performance!) would abruptly
change when maps and graphs were restricted to subsets of their
keys/vertices.
Also adds a generative test for digraph and Graphs merge.
@ztellman ztellman merged commit 3037737 into lacuna:master Nov 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants