Fork me on GitHub
#core-async
<
2021-12-17
>
ns19:12:18

I tried posting this under clojurescript before I was aware that there is a core.async channel as well. It's my first time using core.async with cljs and while the code works and the results I get are fine, the performance is too slow, I'm thinking it might have something to do with how I implemented core.async because the same logic using promises only and handling with .then (without core.async) finishes about 10 times faster. I tried two versions, one using "<p!" macro with promises and another channels only version but performance was similarly bad. It must be that I'm not doing something right when implementing those. I have a list of couple of thousands ids which need to be shortened and stored in firebase. Shortened version is like a human friendly version of the same, but it must be unique.

(def ids ["someListContainingLong80CharacterStringsContaining2000Total"
          ....
          ....])
Already shortened vesions are  written in firebase in 2 collections, long2short and short2long. We query firebase for each id in the list and see which ones are already stored. If it's not stored we send it to function for abbreviation (shorten) which looks for shortest available candidate and writes it to firebase, if the short id is already taken then it increases the length of short id and tries again. This is the first version I tried with "<p!" macro from core.async.interop which takes the value from a promise resolved in function that calls firebase and it worked great but it was too slow for processing couple of thousands of ids (literally minutes of wait time)
(:require
 [cljs.core.async.interop :refer-macros [<p!]])
(go
  (doseq [id (distinct ids)]
    (let [short (<p! (fb/get-long2short id))]
      (when (nil? short)
        (<! (shorten id))))))
(defn shorten [id]
  (go
    (loop [abbr-len 3]
      (let [candidate (subs id 0 abbr-len)
            candidate-id (<p! (fb/get-short2long candidate))]
        (cond
          (empty? candidate-id)
          (do
            (fb/set-short2long candidate id)
            (recur abbr-len))
          (= candidate-id id)
          candidate
          :else
          (recur (inc abbr-len)))))))
(defn get-long2short [id]
  (new js/Promise (fn [resolve _]
                    (.. firebase database (ref (str "long2short/" id))
                        (once "value"
                              (fn success [snapshot]
                                (let [data (-> snapshot .val (js->clj :keywordize-keys true))]
                                  (resolve data))))))))
(defn get-short2long [id]
      ;; same as get-long2short just querying "short2long/" collection
  )
Also I tried couple of versions using channels instead of promises and "<p!" (which from what I understand converts promise to channel anyways) but it didn't help the performance much, it went something like this:
(defn shorten [id]
  (go
    (loop [abbr-len 3]
      (let [candidate (subs id 0 abbr-len)
            c (chan)
            candidate-id (<! (fb/get-short2long candidate c))]
  ...
  )
  and then firebase function success would just put! data on channel:
(put! c data)
I also tried declaring a single channel to be used for this (instead of creating a new one for every shorten function call but it was similar performance wise. Finally I took out core.async stuff and went with promise.then only solution which gave me the perfomance I needed but the code ended up being much uglier. When I run this exact same logic with promises alone it finishes in around 10 seconds while the version with core.async (whether using "<p!" in combination with promises or putting values on channels without promises) takes over 200 seconds. Ideally I would like to get this done to perform fast with core.async, either "<p!" or channels because it would be a lot easier to reason about my code. Any help is appreciated! P.S. Not sure if it matters, I should mention that I did not do production build, I am using shadow cljs and the code was run under watch.

lilactown20:12:43

have you tried profiling your application to see where your code is spending the most time?

phronmophobic21:12:30

I would not expect core.async to be 20x slower here. I would guess that there's something else going on. Some follow up questions: • Is adding thousands of ids something you do regularly? • Does firebase have a batching API? • If you're adding many ids in sequence, it seems like it would be worth it to have multiple in-flight requests at a time (bounded at some number) rather than sending them one-at-a-time. Is there any reason not to do this? • are there possible duplicates in the list? • is the shorten list append only? If so it might make sense to have a local cache. Additionally, depending on the size of the list, you might be able to pre-cache. • Are you running this in the browser? on nodejs?

ns00:12:15

• Is adding thousands of ids something you do regularly? Yes, it's going to run regularly, although in most cases not all ids will be written since some will already be there by the time it runs next time. It may be that I end up with only 10-20% of them written at a time in certain cases and more in others. • Does firebase have a batching API? Yes, haven't used them, but I read somewhere a post from someone at firebase that there are no performance gains using batch write, if anything multiple writes could be faster because they could potentially execute in parallel while batch will execute sequentially. • If you're adding many ids in sequence, it seems like it would be worth it to have multiple in-flight requests at a time (bounded at some number) rather than sending them one-at-a-time. Is there any reason not to do this? Answered above. • are there possible duplicates in the list? Yes, which is why I'm using distinct on ids in that initial loop, everything that ends up in database must be unique. • is the shorten list append only? If so it might make sense to have a local cache. Additionally, depending on the size of the list, you might be able to pre-cache. I'm afraid it would be tough, this list will grow big, and I would be afraid of what one user has stored locally vs. another. • Are you running this in the browser? on nodejs? Running in browser. Backend delivers these ids to cljs app which does shortening so it can display short ids and be more user friendly (long ones are simply too long to work with). When posting to backend they are converted from short to long again.

phronmophobic01:12:09

> • If you're adding many ids in sequence, it seems like it would be worth it to have multiple in-flight requests at a time (bounded at some number) rather than sending them one-at-a-time. Is there any reason not to do this? > Answered above. Does your implementation have multiple requests in-flight at the same time? I might have missed something, but it looks like it's processing the ids one at a time. That could definitely explain the 20x time difference if one implementation was sending multiple requests at the same time and one implementation was only sending a single request at a time. > I'm afraid it would be tough, this list will grow big, and I would be afraid of what one user has stored locally vs. another. I agree that the size of the list might be an issue, but if the list is append only, then it shouldn't matter if the local cache differs across clients. There are ways to trade off size vs having a useful cache. If it's easy to predict which ids a client wants, then you can get a meaningful speedup by using an abbreviated cache that tries to guess what the client wants. > Running in browser. Have you looked at your browser's network inspector? I suspect that its timeline will have some insights as to what might be happening different under the hood. • Is set-short2long a custom firebase function? • Is there a getOrCreate equivalent in firebase or can you add one? It would certainly speed things when shortening. It can also help avoid race conditions • I'm not totally sure how long2short works under the hood, but it doesn't appear like it's using a CAS mechanism to avoid race conditions

ns09:12:06

Believe it or not, the whole issue with performance came down putting "go" block inside of "doseq" instead of outside, so this:

(go
  (doseq [id (distinct ids)]
    (let [short (<p! (fb/get-long2short id))]
      (when (nil? short)
        (<! (shorten id))))))
became this:
(doseq [id (distinct ids)]
  (go    
    (let [short (<p! (fb/get-long2short id))]
      (when (nil? short)
        (<! (shorten id))))))
and the performance went down from 8 minutes to 20 seconds or something like that. I just wish I knew reason for that, I guess I still don't understand how to use go blocks very well. So yes, the performance of version with core.async is just as good as the promise only version. Thank you everyone for your help!

Ben Sless09:12:20

The first version didn't split the work

Ben Sless09:12:10

Think about go blocks as logical threads or processes. You had one process which iterated over all ids. In the second case you had as many processes as ids

ns14:12:09

Yeah, I thought that having go blocks in the loop and creating a process for each id would be costly so I put it outside thinking that having only one go block would be more efficient. Guess I thought wrong.

hiredman21:12:49

what does set-short2long do?

ns00:12:43

set-short2long writes the shortened version of id to firebase

hiredman21:12:28

assuming that writes to firebase, you'll want to be waiting for that write to complete

hiredman21:12:23

(or in cooperative threading lang, yielding the main thread so it can run before continuing)

hiredman21:12:36

it would be interesting to see the promise version to see if it really is doing the same thing or not

hiredman21:12:27

my really kind of vague guess is that the promise code is waiting for set-short2long to run, and because the core.async code isn't, you end up with a lot more network traffic

ns09:12:06

Believe it or not, the whole issue with performance came down putting "go" block inside of "doseq" instead of outside, so this:

(go
  (doseq [id (distinct ids)]
    (let [short (<p! (fb/get-long2short id))]
      (when (nil? short)
        (<! (shorten id))))))
became this:
(doseq [id (distinct ids)]
  (go    
    (let [short (<p! (fb/get-long2short id))]
      (when (nil? short)
        (<! (shorten id))))))
and the performance went down from 8 minutes to 20 seconds or something like that. I just wish I knew reason for that, I guess I still don't understand how to use go blocks very well. So yes, the performance of version with core.async is just as good as the promise only version. Thank you everyone for your help!