Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Fix clojure-emacs/cider#1320] Do state tracking in a future #255

Merged
merged 1 commit into from
Sep 18, 2015

Conversation

Malabarba
Copy link
Member

This should fix all lags associated with the new namespace tracking,
both the long delay at first connection and the ~1sec lag in the REPL.

@Malabarba Malabarba changed the title [Fix #1320] Do state tracking in a future [Fix clojure-emacs/cider#1320] Do state tracking in a future Sep 16, 2015
@bbatsov
Copy link
Member

bbatsov commented Sep 17, 2015

Won't an agent be a better fit for this?

@Malabarba
Copy link
Member Author

Is it ok if the function used to update agents in not pure?

@expez
Copy link
Member

expez commented Sep 17, 2015

I think you're risking retries when using agents (but we only use the agent from one thread), but if the only IO is reading from disk then that should be fine.

@bbatsov
Copy link
Member

bbatsov commented Sep 17, 2015

I think agents never trigger retries, so this should not be a concern. After all people are implementing log systems and things like this using agents. In a transaction an agent won't be executed until the transaction commits, but this is different.

@Malabarba
Copy link
Member Author

I'm running some tests here, and it looks like agents do not trigger retries indeed.

(let [at (agent 0)
      c (atom 0)]
  (dotimes [x 1000]
    (future (Thread/sleep 200)
            (send at #(do (swap! c inc)
                          (inc %)))))
  (Thread/sleep 2000)
  (println @c))

This always prints 1000. Whereas, if I replace (agent 0) with (atom 0) and replace send with swap! then I always get at least 1030.

Not a super rigorous test, but it's convinced me. :)

@Malabarba
Copy link
Member Author

@expez
Copy link
Member

expez commented Sep 17, 2015

I'm not on slack but I just looked at the implementation and you guys are indeed right. There are no retries because the agent holds an ActionQueue internally.

@bbatsov
Copy link
Member

bbatsov commented Sep 17, 2015

So, it seems we have clarity here. Agent it is! :-)

@bbatsov
Copy link
Member

bbatsov commented Sep 17, 2015

Apart from the failing tests the code looks good to me.

This should fix all lags associated with the new namespace tracking,
both the long delay at first connection and the ~1sec lag in the REPL.
bbatsov added a commit that referenced this pull request Sep 18, 2015
@bbatsov bbatsov merged commit 6820553 into master Sep 18, 2015
@bbatsov bbatsov deleted the rewrite-ns-tracker branch September 18, 2015 07:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants