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

Evaluation of Datastore Caching changes #581

Closed
4 tasks done
torao opened this issue Jul 5, 2022 · 1 comment · Fixed by #605
Closed
4 tasks done

Evaluation of Datastore Caching changes #581

torao opened this issue Jul 5, 2022 · 1 comment · Fixed by #605
Assignees

Comments

@torao
Copy link
Contributor

torao commented Jul 5, 2022

Summary

Reconsideration of the data cache mechanism added to the datastore layer.

  • Report on the differences between current lbm-sdk and cosmos-sdk.
    • Aim of the change to lbm-sdk style implementation.
    • The actual effect of the lbm-sdk style implementation.
    • Consideration of the impact of this change on future cosmos-sdk merge task (e.g., how many changes are needed).
  • Decision whether to keep the lbm-sdk implementation or revert to the cosmos-sdk implementation.
  • If revert to a cosmos-sdk implementation, plan and progress of completing that work.

This task is to sort out useful features in order to reduce the differences between lbm-sdk and cosmos-sdk.

Parent: #549


For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate contributors tagged
  • Contributor assigned/self-assigned
@dudong2
Copy link
Contributor

dudong2 commented Jul 22, 2022

Datastore Caching

This is part of the work to reduce the difference between lbm-sdk and cosmos-sdk.
Unlike cosmos-sdk, lbm-sdk performs datastore caching through cache manager in iavl.
It is necessary to determine the validity of the function and, if it is determined that it is not necessary, roll back it for ease of management in the future.

Process

I proceeded to remove the cache manager of iavl. Performance improvements were made in the repository fork cosmos/iavl and tendermint/tm-db. However, there was a problem that the race test failed due to the changed part.
It was necessary to find out how much performance improvement was made in line/iavl and line/tm-db compared to cosmos/iavl and tendermint/tm-db.

Method

A benchmark was performed by executing BankKeeper.SendCoins() for both random and fixed addresses.

package pfm

import (
	"encoding/json"
	"math/rand"
	"strconv"
	"testing"

	abci "github.com/line/ostracon/abci/types"
	"github.com/line/ostracon/libs/log"
	ocproto "github.com/line/ostracon/proto/ostracon/types"
	dbm "github.com/tendermint/tm-db"
	"github.com/stretchr/testify/require"

	"github.com/line/lbm-sdk/simapp"
	sdk "github.com/line/lbm-sdk/types"
)

func BenchmarkBaseAccount(b *testing.B) {
	encCfg := simapp.MakeTestEncodingConfig()
	app := simapp.NewSimApp(log.NewNopLogger(), dbm.NewMemDB(), nil, true, map[int64]bool{}, simapp.DefaultNodeHome, 0, encCfg, simapp.EmptyAppOptions{}, nil)
	genState := simapp.NewDefaultGenesisState(encCfg.Marshaler)
	stateBytes, err := json.MarshalIndent(genState, "", " ")
	require.NoError(b, err)
	app.InitChain(abci.RequestInitChain{
		Validators:      []abci.ValidatorUpdate{},
		ConsensusParams: simapp.DefaultConsensusParams,
		AppStateBytes:   stateBytes,
	})

	ctx := app.NewContext(false, ocproto.Header{})

	addrs := []sdk.AccAddress{}
	addrCount := 100000
	coin := sdk.NewCoins(sdk.NewCoin("atom", sdk.NewInt(1000000000)))
	for ii := 0; ii < addrCount; ii++ {
		addr := sdk.AccAddress("addr" + strconv.Itoa(ii))
		acc := app.AccountKeeper.NewAccountWithAddress(ctx, addr)
		app.AccountKeeper.SetAccount(ctx, acc)
		simapp.FundAccount(app, ctx, addr, coin)
		addrs = append(addrs, addr)
	}

	sendAmount := sdk.NewCoins(sdk.NewCoin("atom", sdk.NewInt(1)))
	b.Run("Testing/IAVLCache", func(b *testing.B) {
		for ii := 0; ii < b.N; ii++ {
			from := rand.Intn(addrCount)
			to := rand.Intn(addrCount)
			// if fixed case:
			// from := 0
			// to := 1

			err = app.BankKeeper.SendCoins(ctx, addrs[from], addrs[to], sendAmount)
		}
	})
	require.NoError(b, err)
}

Result

The following table is the performance of BankKeeper.SendCoins() run with random address and fixed address for the 100,000 accounts. It can be seen that the case of BankKeeper.SendCoins() with cosmos/iavl and tendermint/tm-db is a little faster than that with line/iavl and line/tm-db.

Case Random Addr Fixed Addr
line/iavl, line/tm-db 16,661 ns/op 11,948 ns/op
cosmos/iavl, tendermint/tm-db 16,316 ns/op 11,845 ns/op

Conclusion

I can't think of any reason to keep the our fork versions(line/iavl and line/tm-db) of cosmos/iavl and tendermint/tm-db.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants