baiwyc119 / lxmppd

Automatically exported from code.google.com/p/lxmppd
0 stars 0 forks source link

throttle.lua floor problem #495

Open GoogleCodeExporter opened 9 years ago

GoogleCodeExporter commented 9 years ago
What steps will reproduce the problem?
1. th = throttle.create(1, 1)
2. call th:poll(1) more than once per second
3. these poll call will *never* return true

What is the expected output? What do you see instead?
It is expected that poll will finish to return true, whatever the call 
frequency of :update()

What version of the product are you using? On what operating system?
Any versions on any OSes :)

Please provide any additional information below.
See https://github.com/maranda/metronome/issues/223 for a full description of 
the problem. And please be smarter than Marco Cirillo :D

Original issue reported on code.google.com by pierre.g...@gmail.com on 8 May 2015 at 1:31

GoogleCodeExporter commented 9 years ago
Hi, thanks for the report!

I see the problem. However I'm worried it's not so simple. The floor() was not 
originally there, I added it in this commit: 
https://hg.prosody.im/trunk/rev/0e9a5b63206a

I am quite annoyed at myself for not adding an explanation in a comment or in 
the commit message, but I'm sure there was a reason I added it.

We also don't have any tests for this module, so I'm disinclined to make the 
obvious change without 1) tests or 2) understanding why I added floor() in the 
first place.

But I agree the current behaviour is certainly wrong. Hmm.

I'm setting the milestone as 0.9 because this deserves to be fixed in the 
stable branch.

Original comment by MWild1 on 8 May 2015 at 1:46

GoogleCodeExporter commented 9 years ago
[deleted comment]
GoogleCodeExporter commented 9 years ago
Maybe the floor is there to keep the balance an integer.
What I propose to ensure throttle works as intended, and keep balance an 
integer is:
local growth = self.rate * elapsed;
if growth >= 1 then
    local int_growth = floor(growth);
    self.t = newt - (growth - int_growth) / self.rate;
    local balance = int_growth + self.balance;
    if balance > self.max then
         ...
    ...
-- else dont do anything

Original comment by pierre.g...@gmail.com on 8 May 2015 at 2:41

GoogleCodeExporter commented 9 years ago
Wouldn't just:

-  if balance > self.max then
+  if floor(balance) > self.max then

suffice for that?

Original comment by MWild1 on 8 May 2015 at 2:46

GoogleCodeExporter commented 9 years ago
No I don't think this condition should be changed.

The problem is:
b 0         1 
  |---------|---
t 1         2  ^
               :update() call t=2.3 and balance=1.3 ; but keep balance floored so:
            ^
             \_ back in time here t=2 and balance=1

This is the aim of newt - (growth - int_growth) / self.rate = 2.3 - (0.3)/1 = 2
It keeps internal self.t consistent with balance flooring. Like if update was 
called at t=2

(And obviously if growth < 0 nothing has to be done, just wait for a 
significant delta time)

Original comment by pierre.g...@gmail.com on 8 May 2015 at 2:57

GoogleCodeExporter commented 9 years ago
*growth < 1 sorry

Original comment by pierre.g...@gmail.com on 8 May 2015 at 3:01