Skip to content

Conversation

@it4rb
Copy link
Contributor

@it4rb it4rb commented Jan 24, 2024

InitPoolSimulator take in an instance of pool simulator and fill it instead of allocating new one

Benchmark:
Allocating new simulator every time:
BenchmarkNewPoolSimulator-8 2421100 485.7 ns/op 464 B/op 13 allocs/op

Best case: always re-use the same simulator:
BenchmarkNewPoolSimulatorV2-8 8776808 135.8 ns/op 48 B/op 2 allocs/op

Why did we need it?

Related Issue

Release Note

How Has This Been Tested?

Screenshots (if appropriate):

const NUM_TOKEN = 2

func InitPoolSimulator(entityPool entity.Pool, sim *PoolSimulator) error {
if len(entityPool.Tokens) != NUM_TOKEN || len(entityPool.Reserves) != NUM_TOKEN {
Copy link
Contributor

Choose a reason for hiding this comment

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

just be cautious, we should do nil check with sim here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks! 've added a check below

Comment on lines +84 to +87
var extra Extra
if err := json.Unmarshal([]byte(entityPool.Extra), &extra); err != nil {
return err
}
Copy link
Member

Choose a reason for hiding this comment

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

Can we reuse Extra?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes we could (by putting Extra into the sim struct), but in this case it's quite small so I skipped it (so that I don't have to change NewPoolSimulator v1)

(this case is quite annoying, extra only used inside this function and nothing refer to it after that, but somehow the compiler still decided to moved it to heap instead, the heap analysis show the reason is the json.Unmarshal call, but I'm still not sure why)

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.

4 participants