Skip to content
This repository was archived by the owner on Sep 26, 2023. It is now read-only.

Conversation

@efritz
Copy link
Contributor

@efritz efritz commented Aug 14, 2020

This makes the traversal of packages for the definition, reference, and reference result passes happen concurrently (up to the number of logical CPUs on the index machine).

The logic changes are mostly in visit.go, which now surrounds the loops with a goroutine that instead feeds a channel for runParallel. The remaining work (the majority of changes) is surrounding access of shared datastructures with appropriate mutexes. This is just a sync.[RW]Mutex in most cases, except for the multi-level ranges map, which is guarded by a striped mutex.

This implementation is similar to https://github.com/nmvalera/striped-mutex, but does not require hashing the string key to convert it into a stripe index.

Each commit is a logical group, so reviewers may find it easier to review commit-by-commit rather than look at the resulting diff.

Improvement:

Screen Shot 2020-08-16 at 10 44 00 AM

@efritz efritz changed the title Parallelize passes Parallelize definition, reference, and reference result passes Aug 14, 2020
@efritz efritz requested review from aidaeology and garobrik August 16, 2020 15:44
@efritz efritz marked this pull request as ready for review August 16, 2020 15:45
@efritz efritz removed the request for review from garobrik August 17, 2020 23:17
Copy link
Contributor

@aidaeology aidaeology left a comment

Choose a reason for hiding this comment

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

LGTM; very excited about the speed improvements here 🥇

Base automatically changed from remove-unused-parameters to master August 18, 2020 15:06
@efritz efritz merged commit 7675c11 into master Aug 18, 2020
@efritz efritz deleted the parallelize-passes branch August 18, 2020 15:07
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.

3 participants