Skip to content

feat<memory: priority> add method IncrByFloat, Delete, UpdatePrioriy#28

Merged
flycash merged 5 commits into
ecodeclub:mainfrom
XiaKuan:priority-cache-impl
Oct 31, 2023
Merged

feat<memory: priority> add method IncrByFloat, Delete, UpdatePrioriy#28
flycash merged 5 commits into
ecodeclub:mainfrom
XiaKuan:priority-cache-impl

Conversation

@XiaKuan
Copy link
Copy Markdown
Contributor

@XiaKuan XiaKuan commented Oct 27, 2023

新增rbtree_priority_cache实现Cache的两个方法IncrByFloat, Delete和一个修改优先级的方法UpdatePriority

…y) 新增rbtree_priority_cache实现Cache的两个方法IncrByFloat, Delete和一个修改优先级的方法UpdatePriority
@XiaKuan
Copy link
Copy Markdown
Contributor Author

XiaKuan commented Oct 27, 2023

UpdatePriority 是 #17 实现LRU和LFU需要的方法

@codecov
Copy link
Copy Markdown

codecov Bot commented Oct 28, 2023

Codecov Report

Merging #28 (53cdabc) into main (10ff4bc) will decrease coverage by 0.44%.
The diff coverage is 95.71%.

@@            Coverage Diff             @@
##             main      #28      +/-   ##
==========================================
- Coverage   98.37%   97.94%   -0.44%     
==========================================
  Files           4        4              
  Lines         615      632      +17     
==========================================
+ Hits          605      619      +14     
- Misses          7        9       +2     
- Partials        3        4       +1     
Files Coverage Δ
memory/priority/rbtree_cache_node.go 100.00% <100.00%> (ø)
memory/priority/rbtree_priority_cache.go 99.05% <95.08%> (-0.95%) ⬇️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Comment on lines +354 to +361
node, cacheErr := r.cacheData.Find(key)
if cacheErr != nil {
if r.isFull() {
r.deleteNodeByPriority()
}
node = newFloatRBTreeCacheNode(key)
r.addNode(node)
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

这个地方抽取出来作为一个公共方法,比如说叫做 findOrCreate()。在其它的方法里面也有类似的东西,所以你需要一并重构了

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

重构的话,我觉得你应该用不上这个 newFloat 的方法了

}

// UpdatePriority 更新缓存对象的优先级
func (r *RBTreePriorityCache) UpdatePriority(ctx context.Context, key string, priority int) (bool, error) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

暂时不要暴露这个方法,可以删掉。从实践中来看,需要动态调整优先级的场景还是比较少,我们可以等到用户抱怨,或者说真的需要的时候再提供这个方法。

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

进一步来说,如果要支持这个方法,那么就应该在 Set 方法里面也进一步考虑调整优先级

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

暂时不要暴露这个方法,可以删掉。从实践中来看,需要动态调整优先级的场景还是比较少,我们可以等到用户抱怨,或者说真的需要的时候再提供这个方法。

emm,能不能先把这个方法修改作为内部方法,感觉基于优先级缓存实现类似于lru的淘汰方法,需要一个更新优先级的机制

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

你是打算彻底支持这个动态调整优先级的机制吗?

Copy link
Copy Markdown
Contributor Author

@XiaKuan XiaKuan Oct 29, 2023

Choose a reason for hiding this comment

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

目前的想法是能够设计基于优先级缓存作为客户端,使用类似于装饰器和策略的模式实现lru,lfu以及缓存大小等淘汰方式的缓存
类似于这样

var ErrClientNotMatch = errors.New("不适用的缓存客户端")

type CacheWithPriority interface {
	ecache.Cache
	updatePriority(ctx context.Context, key string, priority int) (bool, error)
}

type EvictedStrategy interface {
	NewElement(val any) (EvictedElement, error)
}

type EvictedElement interface {
	priority.Priority
	GetData() error
	SetData(val any) any
}

type EvictedCache struct {
	cacheClient   CacheWithPriority
	evictedStrage EvictedStrategy
	mutex         *sync.RWMutex
}

func (c *EvictedCache) Set(ctx context.Context, key string, val any, expiration time.Duration) error {
	c.mutex.Lock()
	defer c.mutex.Unlock()

	value := c.cacheClient.Get(ctx, key)
	if value.Err != nil {
		//不应该存在创建实例error
		element, _ := c.evictedStrage.NewElement(val)
		return c.cacheClient.Set(ctx, key, element, expiration)
	}

	element, ok := value.Val.(EvictedElement)
	if !ok {
		return ErrClientNotMatch
	}
        // 更新优先级
	_, _ = c.cacheClient.updatePriority(ctx, key, element.Priority())
	element.SetData(val)

	return c.cacheClient.Set(ctx, key, element, expiration)
}

之后创建一个lru的缓存就可以这样创建和调用,其他的淘汰策略也类似

func NewLruCacheWithRbTree(opts ...option.Option[priority.RBTreePriorityCache]) (*EvictedCache, error) {
	cache, err := priority.NewRBTreePriorityCache(opts...)
	if err != nil {
		return nil, err
	}

	lruCache, err := NewEvictedCache(cache, LruEvictedStrategy{})
	if err != nil {
		return nil, err
	}
	return lruCache, nil
}

type LruEvictedStrategy struct{}

func (l LruEvictedStrategy) NewElement(val any) (EvictedElement, error) {
	return &LruElement{
		data: val,
	}, nil
}

// LruElement 设置缓存元素的方法,当前调用时间作为优先级
type LruElement struct {
	data any
}

func (e *LruElement) GetData() (any, error) {
	return e.data, nil
}

func (e *LruElement) SetData(val any) error {
	e.data = val
	return nil
}

func (e *LruElement) Priority() int {
	return time.Now().Nanosecond()
}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

就感觉自己一下绕不开这个updatePriority了,明哥有没有什么好的建议

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

你是打算彻底支持这个动态调整优先级的机制吗?

不是想支持动态更新优先级的,是想实现lru或者lfu这种类型的缓存淘汰机制,在使用优先级队列缓存的情况下感觉一下子没有想到什么更好的idea

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

你可以看之前的合并请求,原本我们的计划也是希望能够在这个基础上同时支持 LRU 和 LFU,或者说将 LRU 和 LFU 看做是一种特殊的优先级形态,但是最终代码还是过于复杂了。因此最终就是用了这个折中之后的简化实现。

如果你准备实现这个目的,那么你可以尝试一下。注意就是要保证整个实现的可读性。也就是说如果为了支持 LRU 和 LFU,而引入额外的抽象的话,可能不如直接新写一个 LRU 或者 LFU 好。

你可以试试。

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

或者我们可以考虑分成两个步骤。先支持动态调整优先级,不过支持动态调整优先级,就要完整,在 set 的时候要重新计算一下优先级,然后要删除原本的元素,重新插入。

在这个时候,可以适当评估一下为了支持动态调整优先级引入的复杂度以及性能损失。

如果代价不是很高的话,我也很希望达成这个目标。但是我也没特别好的注意。之前我有过一个设想,就是设计一个复杂的数据结构,一个整合了哈希结构和队列结构,或者一个整合了树形结构和队列结构的,甚至于是一个整合了树形结构和跳表结构的复杂数据结构。

我比较孤陋寡闻,不知道有没有相关的数据结构。

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

嗯,这个pr我先删除了更新优先级的相关提交,我再琢磨下有没有相关的数据结构和实现

@flycash
Copy link
Copy Markdown
Contributor

flycash commented Oct 30, 2023

你可以运行一下 make setup,而后再提交。make setup 会设置一些 git hook,可以在提交的时候检测代码,以及运行测试。当然你也可以自己手动运行。

}

// findOrCreateNode 查找节点,不存在时使用默认值创建节点【调用该方法必须先获得锁】
func (r *RBTreePriorityCache) findOrCreateNode(key string, defaultValue any) *rbTreeCacheNode {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

这边改进一下设计,不再是传入一个 defaultValue,而是传入一个 func,也就是说,你通过调用这个方法来获得这个 defaultVal。这样可以避免在 Find 命中的时候也要初始化 defaultValue。其余就没什么问题了。

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

可以可以学到了,很细 o( ̄▽ ̄)d

@XiaKuan
Copy link
Copy Markdown
Contributor Author

XiaKuan commented Oct 31, 2023

你可以运行一下 make setup,而后再提交。make setup 会设置一些 git hook,可以在提交的时候检测代码,以及运行测试。当然你也可以自己手动运行。

这个我加上了,这个报错对应的是autoClean和Get的代码;应该是这一小段的node没有加锁导致的
image

但是很神奇这段代码在这个pr里面没有修改过,是之前遗留的但是单测都好像通过了,是不是go test -race ./... 有概率测不出来。。。。

@flycash
Copy link
Copy Markdown
Contributor

flycash commented Oct 31, 2023

一般都不是 test -race 出了问题,而是在测试的时候没有注意保护。这种错漏很常见。

@flycash flycash merged commit 8c6eedc into ecodeclub:main Oct 31, 2023
@XiaKuan XiaKuan deleted the priority-cache-impl branch November 1, 2023 07:13
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.

2 participants