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.

Sign up for GitHub

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

Jump to bottom

core, trie, rpc: speed up tests #28461

Merged
merged 7 commits into from
Nov 21, 2023
Merged

core, trie, rpc: speed up tests #28461

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
b051500
rpc: make subscription test faster
MariusVanDerWijden Nov 3, 2023
c413663
trie: cache trie nodes for faster sanity check
MariusVanDerWijden Nov 3, 2023
cb349b0
core/forkid: speed up validation test
MariusVanDerWijden Nov 3, 2023
be78565
core/state: improve snapshot test run
MariusVanDerWijden Nov 3, 2023
62a3291
accounts/keystore: improve keyfile test
MariusVanDerWijden Nov 3, 2023
08be7f1
trie: remove resolver
MariusVanDerWijden Nov 15, 2023
65bebf3
trie: only check ~5% of all trie nodes
MariusVanDerWijden Nov 15, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion rpc/client_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -595,7 +595,7 @@ func TestClientSubscriptionChannelClose(t *testing.T) {

for i := 0; i < 100; i++ {
ch := make(chan int, 100)
sub, err := client.Subscribe(context.Background(), "nftest", ch, "someSubscription", maxClientSubscriptionBuffer-1, 1)
sub, err := client.Subscribe(context.Background(), "nftest", ch, "someSubscription", 100, 1)
Copy link
Contributor

Choose a reason for hiding this comment

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

This change makes a huge difference in execution speed, however, the semantics of this test is pretty complicated. Would be good if @fjl checks that the test semantics are still maintained.

Copy link
Member Author

@MariusVanDerWijden MariusVanDerWijden Nov 6, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

As far as I understood the test it just opens a lot of subscriptions and subscribes to a lot of messages, then shuts down the subscriptions in order to catch a racy error

Copy link
Contributor

Choose a reason for hiding this comment

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

@fjl pls ack if this LGTY?

Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry for the slow response. It's OK to change this.

if err != nil {
t.Fatal(err)
}
Expand Down
29 changes: 24 additions & 5 deletions trie/sync_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ func makeTestTrie(scheme string) (ethdb.Database, *Database, *StateTrie, map[str
func checkTrieContents(t *testing.T, db ethdb.Database, scheme string, root []byte, content map[string][]byte, rawTrie bool) {
// Check root availability and trie contents
ndb := newTestDatabase(db, scheme)
if err := checkTrieConsistency(db, scheme, common.BytesToHash(root), rawTrie); err != nil {
if err := checkTrieConsistency(db, scheme, common.BytesToHash(root), rawTrie, nil); err != nil {
t.Fatalf("inconsistent trie at %x: %v", root, err)
}
type reader interface {
Expand Down Expand Up @@ -101,7 +101,7 @@ func checkTrieContents(t *testing.T, db ethdb.Database, scheme string, root []by
}

// checkTrieConsistency checks that all nodes in a trie are indeed present.
func checkTrieConsistency(db ethdb.Database, scheme string, root common.Hash, rawTrie bool) error {
func checkTrieConsistency(db ethdb.Database, scheme string, root common.Hash, rawTrie bool, resolver NodeResolver) error {
ndb := newTestDatabase(db, scheme)
var it NodeIterator
if rawTrie {
Expand All @@ -117,6 +117,9 @@ func checkTrieConsistency(db ethdb.Database, scheme string, root common.Hash, ra
}
it = trie.MustNodeIterator(nil)
}
if resolver != nil {
it.AddResolver(resolver)
}
for it.Next(true) {
}
return it.Error()
Expand Down Expand Up @@ -512,8 +515,8 @@ func testDuplicateAvoidanceSync(t *testing.T, scheme string) {
// Tests that at any point in time during a sync, only complete sub-tries are in
// the database.
func TestIncompleteSyncHash(t *testing.T) {
testIncompleteSync(t, rawdb.HashScheme)
testIncompleteSync(t, rawdb.PathScheme)
testIncompleteSync(t, rawdb.HashScheme) // 9.585s
testIncompleteSync(t, rawdb.PathScheme) // 19.522
}

func testIncompleteSync(t *testing.T, scheme string) {
Expand Down Expand Up @@ -585,16 +588,32 @@ func testIncompleteSync(t *testing.T, scheme string) {
})
}
}

// Cache trie nodes for faster verification
nodesCache := make(map[string][]byte)
resolver := func(owner common.Hash, path []byte, hash common.Hash) []byte {
if scheme == rawdb.HashScheme {
return nodesCache[string(hash[:])]
}
return nodesCache[string(path)]
}

// Sanity check that removing any node from the database is detected
for i, path := range addedKeys {
owner, inner := ResolvePath([]byte(path))
nodeHash := addedHashes[i]
value := rawdb.ReadTrieNode(diskdb, owner, inner, nodeHash, scheme)
rawdb.DeleteTrieNode(diskdb, owner, inner, nodeHash, scheme)
if err := checkTrieConsistency(diskdb, srcDb.Scheme(), root, false); err == nil {
if err := checkTrieConsistency(diskdb, srcDb.Scheme(), root, false, resolver); err == nil {
MariusVanDerWijden marked this conversation as resolved.
Show resolved Hide resolved
t.Fatalf("trie inconsistency not caught, missing: %x", path)
}
rawdb.WriteTrieNode(diskdb, owner, inner, nodeHash, value, scheme)
// We only add nodes to the cache that we already visited
if scheme == rawdb.HashScheme {
nodesCache[string(nodeHash[:])] = value
} else {
nodesCache[string(inner[:])] = value
}
}
}

Expand Down