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

Parallelise checkUsingKeys #231

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Conversation

neilalexander
Copy link
Contributor

This speeds up signature checks by performing them across available CPU cores (less one, to not totally starve the system).

This might cause problems if lots of big signature checks are taking place concurrently though, so there's that.

"gomatrixserverlib: key with ID %q for %q not valid at %d",
keyID, j.request.ServerName, j.request.AtTS,
)
mu.Unlock()
Copy link
Member

@kegsay kegsay Oct 5, 2020

Choose a reason for hiding this comment

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

I'm not a fan of the shared variables across goroutines, it feels like a code smell as opposed to sharing data via channels. I'm aware we do this to maintain the index in the results array.

I think a better solution would be to add the parallelisation around VerifyJSON only, meaning you just need to pass information that is directly used without needing to access shared slices/maps. Something like:

type verifyJSONReq struct {
    i int
    serverName ServerName
    keyID KeyID
    pubKey ed25519.PublicKey
    msg []byte
}
type verifyJSONRes struct {
    i int
    err error
}
reqCh := make(chan verifyJSONReq, 50)
resCh := make(chan verifyJSONRes, 50)
var wg sync.WaitGroup
wg.Add(procs)
for i := 0; i < procs; i++ {
    go func() {
        defer wg.Done()
        for item := range reqCh {
             err := VerifyJSON(item.serverName, item.keyID, item.pubKey, item.msg)
            resCh <- verifyJSONRes{item.i, err}
        }
    }
}
for i := range requests {
    // insert code that was there previously up to VerifyJSON
    reqCh <- verifyJSONReq{i, other, func, args}
}
close(reqCh) // kill the goroutines after they process everything
// kill the response channel when we've got all the response
go func() {
    wg.Wait()
    close(resCh)
}()
for res := range resCh {
    results[res.i].Error = res.err
}

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