Fork me on GitHub
#cljs-dev
<
2016-04-11
>
Alex Miller (Clojure team)00:04:30

@roman01la: I saw your CA and have added the edit roles

darwin16:04:48

I’m wondering why is core.async timeout implemented using SkipList[1]. I would expect implementation using javascript setTimeout function would be straightforward and just few lines of code. I’m asking because I started to rely on timeout heavily and sometimes my timeout channels close early without apparent reason (non-deterministic). Maybe I hit some bug in SkipList implementation. [1] https://github.com/clojure/core.async/blob/master/src/main/clojure/cljs/core/async/impl/timers.cljs

dnolen16:04:37

@darwin timeouts coalesce if closer than 10ms

dnolen16:04:11

same as the Clojure implementation

darwin16:04:45

ok, I’m going to implement my dumb version using setTimeout to see if my problems go away

darwin16:04:50

then I will investigate it further

darwin16:04:15

that non-deterministic fashion of those unexpected closes is really hard to debug 😕

darwin17:04:54

the problem happens when I have a lot of timers “in-flight” and then refresh the page and it creates bunch of more timers, and then I refresh again, second, third or fourth refresh usually has a failing timeout which closes early (like after 100ms-200ms, instead of 5000ms)

darwin17:04:23

I’m sure all timeouts were passed 5000

dnolen17:04:54

it doesn’t matter what they were passed

dnolen17:04:02

it matters if they will fire within 10ms of each other

darwin17:04:03

ok, I was just speaking that I’m sure I’m passing proper time-limit in msec for them to close

darwin17:04:40

I will now do an experiment and not pass 5000 all the time, I will pass numbers 100ms apart from each other, to prevent coallescing

dnolen17:04:45

note the coalescing is an optimization

dnolen17:04:58

while you can experiment with this, rest assured I spent ridiculous amounts of time on this problem

dnolen17:04:20

if you fire lots of setTimeouts you will get worse behavior across the board

dnolen17:04:28

just testing one browser isn’t good enough for this kind of testing

darwin17:04:58

I agree that the optimization makes sense, and I understand your motivation

darwin17:04:21

but there might be a bug, I’m going to uncover (probably this afternoon :)

dnolen17:04:22

also note we don’t even use setTimeout if can avoid it

dnolen17:04:41

it has the worst behavior and performance characteristics of the various ways to get into the event queue

darwin17:04:12

ok, when I make sure that my timeouts are descending and 100ms apart each other, the problem goes away

darwin17:04:07

so it seems my problem lies in that coalescing logic, when I create multiple timeouts in quick order with the same msec parameter and they get coallesced

dnolen17:04:47

yep that’s just how it works

darwin17:04:14

aha, so that coallescing is not transparent to the timeout user?

darwin17:04:21

I thought it was just an optimization

dnolen17:04:41

it’s not something that can be configured or controlled in any official way

dnolen17:04:06

if you want to see that start a discussion on clojure-dev mailing list

taylor.sando17:04:13

What's the recommended way of getting setTimeout behavior?

dnolen17:04:31

core.async uses goog.async.nextTick

darwin17:04:16

can you explain why coallescing leads to unexpected channel close before timeout interval? I still don’t understand why it behaves this way. I would expected coallesced events would share a channel, but would still work as expected (closed after 5000ms in my case even if the channel returned from timeout call can be potentially shared with someone else)

darwin18:04:19

ok, now I understand what went wrong. I had a completely unrelated code which was creating a timeout channel and closing it when it was no longer needed (after receiving a message "in time"). In those rare occasions when someone else created timeout which by coincidence coalesced with that one bad things started to happen. Forcible close of the channel closes unexpectedly the channel for others BEFORE timeout elapsed.

darwin18:04:02

lesson learned: never ever close! channel received from (timeout …) calls, it is unsafe

darwin18:04:49

this one was tricky to track down, because when I added console.log statements, or reshaped code a bit, the issue was no longer reproducible

dnolen20:04:57

@darwin you might want to confirm that the behavior is actually the same between Clojure & ClojureScript - it may be a bug in ClojureScript

dnolen20:04:08

that is the coalescing part is just a red herring

darwin20:04:56

well, if the channel is reused, how can the library tell who called the close!, close will simply close the channel for all who share it

darwin20:04:35

thanks for your comments and help on this

darwin20:04:25

I didn’t initially understand that coalescing can have effect on me as a user, I didn’t get it, because the model in my head was “it is just an implementation detail - optimization"

dnolen20:04:25

right but from one perspective calling close on a timeout channel seems possibly strange, the coalescing stuff aside

dnolen20:04:28

esp. if it was shared somehow - since that can’t possibly be what you want

dnolen20:04:38

but I agree that this implicit sharing can be unexpected

darwin20:04:51

my motivation was to clear out timeout channels which were no longer needed

dnolen20:04:00

but closing doesn’t have anything to do with “clearing out"

darwin20:04:19

well, my naive model in my head was “there is a table of all timeouts” which holds references to future timeouts, if I close them and forget, it will get garbage-collected

dnolen20:04:35

right that’s not the right mental model

dnolen20:04:38

it will get GCed anyway

dnolen20:04:44

closing doesn’t have anything to do with GC

dnolen20:04:09

closing is very much a special case since everyone blocked on reads will immediately progress

dnolen20:04:23

this is very different from writing to a channel, only one reader will progress

darwin20:04:07

so how can I possibly “cancel” a scheduled timeout if I know I don’t want to handle it anyways and I want to garbage-collect it ASAP, I just forget about it, there is no”callback" which will fire at some point in the future?

darwin20:04:57

that is hopefully safe, but still I think the original reasoning wasn’t obviously wrong, I got a channel, so I close it when I don’t need it anymore

darwin20:04:03

the tricky part was that this worked most of the time

dnolen20:04:59

you shouldn’t close channel when you don’t need them anymore

dnolen20:04:04

it doesn’t serve any purpose

dnolen20:04:27

you only close channels if you want to communicate something to people who might be reading it