Terry-Mao / goim

goim
https://goim.io/
MIT License
7.2k stars 1.78k forks source link

Timer 代码中 up函数的问题 #282

Closed pslzym closed 5 years ago

pslzym commented 5 years ago

各位作者好: 我在看代码的时候有个疑问点,想请教一下。 在添加定时器的时候,会调用这个up函数。但是up函数的逻辑是如果传入的j是大于等于0的话,就什么都不执行,相当于 swap和j=i都不会执行。 我看了所有调用这个up的函数,j传过来基本都是大于等于0的。比如下面的add函数,j为数组的len,不可能小于0。del的函数正常情况下也基本不会小于0。 请问这个地方是有什么其他的作用吗? 还是我代码没有看完整,没有完全理解代码的意思?

`func (t *Timer) up(j int) { for { i := (j - 1) / 2 // parent if i <= j || !t.less(j, i) { break } t.swap(i, j) j = i } }

func (t Timer) add(td TimerData) { var d itime.Duration td.index = len(t.timers) // add to the minheap last node t.timers = append(t.timers, td) t.up(td.index) if td.index == 0 { // if first node, signal start goroutine d = td.Delay() t.signal.Reset(d) if Debug { log.Infof("timer: add reset delay %d ms", int64(d)/int64(itime.Millisecond)) } } if Debug { log.Infof("timer: push item key: %s, expire: %s, index: %d", td.Key, td.ExpireString(), td.index) } }

func (t Timer) del(td TimerData) { var ( i = td.index last = len(t.timers) - 1 ) if i < 0 || i > last || t.timers[i] != td { // already remove, usually by expire if Debug { log.Infof("timer del i: %d, last: %d, %p", i, last, td) } return } if i != last { t.swap(i, last) t.down(i, last) t.up(i) } // remove item is the last node t.timers[last].index = -1 // for safety t.timers = t.timers[:last] if Debug { log.Infof("timer: remove item key: %s, expire: %s, index: %d", td.Key, td.ExpireString(), td.index) } }`

oasangqi commented 5 years ago

我也觉得奇怪,所以我提交了一次 https://github.com/Terry-Mao/goim/pull/271/commits/050b8649e8b67a519f9afa1b2c4b4eed6f23a3ff

lvphpwb commented 5 years ago

这个地方应该是一个有序的切片,插入一个新元素后还需要保持有序,感觉有点问题,应该需要用二分法找到插入的位置,然后做切片的拼装

Terry-Mao commented 5 years ago

Timer 其实可以移除了,Go 新版本对Timer做过大锁优化了

pslzym commented 5 years ago

Timer 其实可以移除了,Go 新版本对Timer做过大锁优化了

非常感谢作者的回答。我其实是想弄明白这段代码这么写的逻辑。可能有些人还用着这个版本的代码,如果作者有时间的话,可以帮忙确认一下,这个地方是否是有问题的。 如果确认是有的话,我可以协助修改相关的代码。

谢谢

tonybase commented 5 years ago

代码其实没问题的,可以看看http://mckee.cn/golang/goim-timer