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

perf: use fast unsafe bytes->string convertion #525

Merged
merged 9 commits into from
Aug 3, 2022
Prev Previous commit
Next Next commit
add fast convertions to cache
  • Loading branch information
robert-zaremba committed Jul 25, 2022
commit 5c367030a19c28d916d43dff4b67ae49a9b5b62b
14 changes: 8 additions & 6 deletions cache/cache.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ package cache

import (
"container/list"

ibytes "github.com/cosmos/iavl/internal/bytes"
)

// Node represents a node eligible for caching.
Expand Down Expand Up @@ -59,15 +61,15 @@ func New(maxElementCount int) Cache {
}

func (c *lruCache) Add(node Node) Node {
if e, exists := c.dict[string(node.GetKey())]; exists {
if e, exists := c.dict[ibytes.UnsafeBytesToStr(node.GetKey())]; exists {
c.ll.MoveToFront(e)
old := e.Value
e.Value = node
return old.(Node)
}

elem := c.ll.PushFront(node)
c.dict[string(node.GetKey())] = elem
c.dict[ibytes.UnsafeBytesToStr(node.GetKey())] = elem

if c.ll.Len() > c.maxElementCount {
oldest := c.ll.Back()
Expand All @@ -78,15 +80,15 @@ func (c *lruCache) Add(node Node) Node {
}

func (nc *lruCache) Get(key []byte) Node {
if ele, hit := nc.dict[string(key)]; hit {
if ele, hit := nc.dict[ibytes.UnsafeBytesToStr(key)]; hit {
nc.ll.MoveToFront(ele)
return ele.Value.(Node)
}
return nil
}

func (c *lruCache) Has(key []byte) bool {
_, exists := c.dict[string(key)]
_, exists := c.dict[ibytes.UnsafeBytesToStr(key)]
return exists
}

Expand All @@ -95,14 +97,14 @@ func (nc *lruCache) Len() int {
}

func (c *lruCache) Remove(key []byte) Node {
if elem, exists := c.dict[string(key)]; exists {
if elem, exists := c.dict[ibytes.UnsafeBytesToStr(key)]; exists {
Copy link
Member

Choose a reason for hiding this comment

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

I'm surprised that this is helpful because, from my understanding, copying during []byte to string conversion when accessing a map by key should be optimized by the Go compiler.

Sources:

Copy link
Member

Choose a reason for hiding this comment

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

I'm wondering if the benchmark would remain the same as it is right now if the map[string] changes are reverted

Copy link
Collaborator Author

@robert-zaremba robert-zaremba Aug 2, 2022

Choose a reason for hiding this comment

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

I can check, however for consistency I prefer to keep the casting here.

return c.remove(elem)
}
return nil
}

func (c *lruCache) remove(e *list.Element) Node {
removed := c.ll.Remove(e).(Node)
delete(c.dict, string(removed.GetKey()))
delete(c.dict, ibytes.UnsafeBytesToStr(removed.GetKey()))
return removed
}