Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Speed up persisting large number of outliers #16649

Merged
merged 2 commits into from
Nov 16, 2023

Conversation

erikjohnston
Copy link
Member

Recalculating the roots tuple every iteration could be very expensive, so instead let's do a topological sort.

(Note that sorted_topologically_batched is basically a slightly modified sorted_topologically, and the tests are more or less a copy and paste)

@erikjohnston erikjohnston force-pushed the erikj/faster_persisting_auth_chain branch from be10458 to 2441b37 Compare November 16, 2023 12:55
@erikjohnston erikjohnston marked this pull request as ready for review November 16, 2023 12:59
@erikjohnston erikjohnston requested a review from a team as a code owner November 16, 2023 12:59
@@ -135,3 +135,54 @@ def sorted_topologically(
degree_map[edge] -= 1
if degree_map[edge] == 0:
heapq.heappush(zero_degree, edge)


def sorted_topologically_batched(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FWIW I think sorted_topologically could become

def sorted_topologically(nodes, graph):
	for batch in sorted_topologically_batched(nodes, graph):
        yield from batch

It would probably create a bunch of intermediate lists and such though, which is unnecessary.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, ish. I think sorted_topologically gives a lexicographical topological sort, which could return a slightly different order (though I don't think we really rely on that fact anywhere, so)

@erikjohnston erikjohnston merged commit 1b238e8 into develop Nov 16, 2023
41 checks passed
@erikjohnston erikjohnston deleted the erikj/faster_persisting_auth_chain branch November 16, 2023 14:25
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants