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

Quorum intersection checker might benefit from caching subproblems #2140

Closed
graydon opened this issue Jun 12, 2019 · 1 comment
Closed

Quorum intersection checker might benefit from caching subproblems #2140

graydon opened this issue Jun 12, 2019 · 1 comment
Assignees
Labels
cleanup refactoring or other internal improvements enhancement
Projects

Comments

@graydon
Copy link
Contributor

graydon commented Jun 12, 2019

There are quite a lot of subproblems so it's not entirely clear that dynamic programming would be a win (or a win in all cases) but it's worth investigating.

@graydon graydon added enhancement cleanup refactoring or other internal improvements labels Jun 12, 2019
@MonsieurNicolas MonsieurNicolas added this to To do in v15.0.0 Sep 23, 2020
@hidenori-shinohara
Copy link
Contributor

hidenori-shinohara commented Oct 15, 2020

I tried caching results of

(You can see the implementation of each by clicking it. Apologies for the not-so-good-looking implementations. I figured that I'd start with something quick that may not look very pretty just to see how much faster it will be.)

I compared the speed by running time (src/stellar-core test -a 'quorum intersection scaling test') 5 times and calculating the median and average of those 5 runs.

Results

  • Both containsQuorumSliceForNode and containsQuorumSlice ended up at least 10x slower.
  • Both isAQuorum and contractToMaximalQuorum ended up being roughly 5% faster.

Observations

  • In most test cases, the function was only able to reuse cached results 30-70% of the time because not so many problems showed up many times.
  • It was surprising to see that containsQuorumSliceForNode and containsQuorumSlice ended up being 10x slower. The only explanation that I could come up with was that turning a BitSet into a key & looking it up may not be that much cheaper than just checking if there exists a quorum slice contained in the BitSet especially if most quorum slices are indeed in the given node set.
  • I'd imagine that there's a better implementation to cache subproblems, but from this, I couldn't really figure out a way to make the checker that much faster.

@MonsieurNicolas MonsieurNicolas removed this from To do in v15.0.0 Oct 21, 2020
@MonsieurNicolas MonsieurNicolas added this to To do in v15.1.0 via automation Oct 21, 2020
@MonsieurNicolas MonsieurNicolas moved this from To do to In progress in v15.1.0 Oct 21, 2020
@MonsieurNicolas MonsieurNicolas moved this from In progress to Done in v15.1.0 Nov 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cleanup refactoring or other internal improvements enhancement
Projects
No open projects
Development

No branches or pull requests

3 participants