A bug using .seconds in throttle.

[danielt1263]

Short description of the issue:

I found a throttle bug.

_ = Observable<Void>.merge(Observable.just(()), Observable.just(()).delay(.milliseconds(500), scheduler: MainScheduler.instance))
	.throttle(.seconds(1), latest: false, scheduler: MainScheduler.instance)
	.debug("Doesn't Work")
	.subscribe()


_ = Observable<Void>.merge(Observable.just(()), Observable.just(()).delay(.milliseconds(500), scheduler: MainScheduler.instance))
	.throttle(.milliseconds(1000), latest: false, scheduler: MainScheduler.instance)
	.debug("works")
	.subscribe()

Expected outcome:

Each of the two observable chains above should emit a single next event.

What actually happens:

The "Doesn't Work" chain emits two events.

RxSwift/RxCocoa/RxBlocking/RxTest version/commit

5.0.0

Platform/Environment

• iOS
• macOS
• tvOS
• watchOS
• playgrounds

How easy is to reproduce? (chances of successful reproduce after running the self contained code)

• easy, 100% repro
• sometimes, 10%-100%
• hard, 2% - 10%
• extremely hard, %0 - 2%

Xcode version:

11.1

Installation method:

• CocoaPods
• Carthage
• Git submodules

I have multiple versions of Xcode installed:
(so we can know if this is a potential cause of your issue)

• yes (which ones)
• no

Level of RxSwift knowledge:
(this is so we can understand your level of knowledge
and formulate the response in an appropriate manner)

• just starting
• I have a small code base
• I have a significant code base

[kzaher]

Hi @danielt1263,

your code doesn't prove that we have a bug. It seems to me that you have defined a race in your code between throttle and delay.

There is no guarantee regarding timing in these cases. We are relying on the underlying dispatch queue behavior mostly.

If you think that something can be improved, you are welcome to make a PR so we can discuss your proposal, but if you presented this code to someone without "works" and "doesn't work" nobody could tell you what is expected from that code IMHO.

[freak4pc]

Hey @danielt1263 — would appreciate your comment on this thread, if you could provide a more practical and non-theoretical example. We unfortunately can't control GCD and its relation to DispatchTime, but this is still interesting if you have more content to share.

[danielt1263]

-- Edit: The last paragraph is actually the most important if you want to skip to that.

Sure. In the actual code where I discovered the issue, the first Observable.just() is actually rx.methodInvoked(#selector(viewDidAppear(_:))).map { _ in } (in my view controller's viewDidLoad) while the Observable.just(()).delay(.milliseconds(500), scheduler: MainScheduler.instance) is actually refreshControl.rx.controlEvent(.valueChanged).asObservable()

It's surprising to me that two events can come through with a full half-second delay between them and throttle(.seconds(1)) doesn't throttle the latter event (whereas throttle(.milliseconds(1000)) does.

I get that you can't control exactly when events are scheduled in relation to each other, but for the throttle to miss a full half-second delay is odd isn't it?

Note that given the following:

_ = Observable.merge(
		Observable.just(()).concat(Observable<Void>.never()),
		Observable.just(()).delay(.milliseconds(500), scheduler: MainScheduler.instance).concat(Observable<Void>.never())
)
	.debug("input")
	.throttle(.seconds(1), latest: false, scheduler: MainScheduler.instance)
	.debug("output")
	.subscribe()

I'm getting output of:

2020-01-05 15:28:18.440: output -> subscribed
2020-01-05 15:28:18.441: input -> subscribed
2020-01-05 15:28:18.443: input -> Event next(())
2020-01-05 15:28:18.443: output -> Event next(())
2020-01-05 15:28:18.944: input -> Event next(())
2020-01-05 15:28:18.944: output -> Event next(())

The first event enters throttle at 15:28:18.443 and the second one enters at 15:28:18.944 yet the operator doesn't know to throttle the second event.

Note that if you set the delay to 400 milliseconds, then it does throttle the event, but it fails to throttle if the delay is at 500 milliseconds or greater. I get the impression that there is some sort of integer division going on when there shouldn't be.

[freak4pc]

I'm closing every issue older than 6 months. If this is still an issue for you, please comment and I can reopen :) Thanks !

[huangqingchao]

我觉得这里是有点问题，主要在代码的这个位置

internal func reduceWithSpanBetween(earlierDate: Date, laterDate: Date) -> DispatchTimeInterval {
        return self.map { value, factor in
            let interval = laterDate.timeIntervalSince(earlierDate)
            let remainder = Double(value) - interval * factor
            guard remainder > 0 else { return 0 }
            return Int(remainder.rounded(.toNearestOrAwayFromZero))
        }
    }

这里是对比 上一次触发的时间 和 当前的时间，如果计算后最后返回值是0，那么就触发一次。
但是这里最后使用了四舍五入。
假如传入的参数是 .second(1)
上次触发的时间戳是 00001.0
当前的时间戳是 00001.6
那么差值为 0.4 秒 (Double(1) - (1.6 - 1.0))，执行最后一行四舍五入的代码，就会导致返回0，直接触发。
这就是有0.499999...秒误差的原因

[jolyot]

@freak4pc
As @huangqingchao mentioned, the rounding in the last line of the reduceWithSpanBetween(earlierDate:laterDate:) function is the cause.

RxSwift/RxSwift/Date+Dispatch.swift

Lines 43 to 50 in 65bec4c

	internal func reduceWithSpanBetween(earlierDate: Date, laterDate: Date) -> DispatchTimeInterval {
	return self.map { value, factor in
	let interval = laterDate.timeIntervalSince(earlierDate)
	let remainder = Double(value) - interval * factor
	guard remainder > 0 else { return 0 }
	return Int(remainder.rounded(.toNearestOrAwayFromZero))
	}
	}

For example, if .second(1) is set in the code below, a quick consecutive tap may cause events to occur in 0.5 second increments for the above reason.

button.rx.tap.throttle(.seconds(1), scheduler: MainScheduler.instance)

This is probably not the intended behavior in many cases, so it might be better to describe this in the documentation or something.

Please consider this if you would like.

[danielt1263]

Okay, so the DispatchTimeInterval represents both the time and resolution. I always wondered why they held Ints instead of doubles...

I think we can leave this closed, but it would be nice to document somewhere...