Skip to content

Conversation

@yasirfolio3
Copy link
Contributor

Summary

  • Added an interface for LRU Caching mechanism. Include a default implementation.
  • Generics were not used since they were introduced in go 1.18 whereas we provide support up-to go 1.12.

Test plan

Included new tests in the test project

Issues

FSSDK-8513

Copy link

@jaeopt jaeopt left a comment

Choose a reason for hiding this comment

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

Looks great - with a clarification

}

// NewLRUCache returns a new instance of Least Recently Used in-memory cache
func NewLRUCache(size int, timeoutInSecs int64) LRUCache {
Copy link

Choose a reason for hiding this comment

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

int64 to int? Range will be less than ~10000

Copy link
Contributor Author

Choose a reason for hiding this comment

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

To avoid converting timeoutInSecs to in64 every time isValid is called since time.Now().Unix() returns a int64 value and calculations are only allowed between values of similar type's.

Copy link
Contributor

@msohailhussain msohailhussain left a comment

Choose a reason for hiding this comment

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

please address my comments and lgtm!!!

Reset()
}

type cacheElement struct {
Copy link
Contributor

Choose a reason for hiding this comment

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

Cache ?


// Cache is used for caching ODP segments
type Cache interface {
Save(key string, value interface{})
Copy link
Contributor

Choose a reason for hiding this comment

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

can we add specific type of interface instead of interface{}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

interface has no further types in go.

l.lock.Lock()
defer l.lock.Unlock()
if item, ok := l.items[key]; !ok {
if l.maxSize == len(l.items) {
Copy link
Contributor

Choose a reason for hiding this comment

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

add comment. what this logic does.

Copy link
Contributor

@msohailhussain msohailhussain left a comment

Choose a reason for hiding this comment

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

lgtm

@msohailhussain msohailhussain merged commit df1036c into master Oct 20, 2022
@msohailhussain msohailhussain deleted the yasir/lru-cache branch October 20, 2022 05:10
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