Skip to content

Commit

Permalink
Implement cancellable tasks for all expensive functions
Browse files Browse the repository at this point in the history
Fixes #143
  • Loading branch information
jpmonettas committed Mar 14, 2024
1 parent 0279673 commit b34891e
Show file tree
Hide file tree
Showing 24 changed files with 790 additions and 500 deletions.
11 changes: 11 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,17 @@
### New Features

- Implemented thread trace limit as a fuse for infinite loops/recursion
- All possibly expensive slow operations are now cancellable. This includes :
- List functions calls
- List prints with the printer
- Values search
- Multi-thread timeline
- All power stepping tools
- Quick jump
- All functions that collect from the timeline will report as they run, no need to wait to the end. This includes :
- List functions calls
- List prints with the printer
- Multi-thread timeline

### Changes
Expand Down
23 changes: 21 additions & 2 deletions docs/dev_notes.md
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ It is currently implemented as `flow-storm.runtime.indexes.timeline-index/Execut
uses a mutable list. It would be simpler if this could be an immutable list but the decision was made because it needs
to be fast to build, and without too much garbage generation, so we don't make the debuggee threads too slow.
With the current architecture transients can't be used because there isn't a trace that indicates that a thread is done,
so it can be persisted. Maybe it can be done in the future if some kind of batching by time is implemented.
so it can't be persisted. Maybe it can be done in the future if some kind of batching by time is implemented.

All objects that represent traces are defined by types in `flow-storm.runtime.types.*` instead of maps. This is to
reduce memory footprint.
Expand Down Expand Up @@ -179,7 +179,26 @@ If there is nothing subscribed to runtime events the events will accumulate insi
This is to capture events fired by recording when the __debugger__ is still not connected.

On the __debugger__ side they will accumulate on `flow-storm.debugger.events-queue` and will be dispatched by a
specialized thread.
specialized thread. Most events are processed by `flow-storm.debugger.events-processor/process-event` but any part
of the __debugger__ can listen to __runtime__ events by adding a callback with `flow-storm.debugger.events-queue/add-dispatch-fn`.

### Tasks

Most of the __runtime__ functionality the __debugger__ calls is called synchronously, but there are some functions where doing
like this leads to suboptimal UX. This are most functionality that needs to search or collect on the timeline. For big recordings
this could take a long time.
For this reason, functions that loop on the timeline run under tasks. This are looping process that run async, can report progress
and can be interrupted.

All the functions that run tasks ends up in `-task`, like `search-next-timeline-entry-task`.
From the __debugger__, for calling a task function `flow-storm.debugger.ui.tasks/submit-task` can be used.

On the __runtime__ side, there are a couple of utilities that make implementing this interruptible loopings easies :

- `flow-storm.runtime.debuggers-api/submit-batched-collect-interruptible-task` (for collecting functionality that traverse the entire timeline)
- `flow-storm.runtime.debuggers-api/submit-find-interruptible-task` (for looping through first match)

There

### Remote debugging

Expand Down
7 changes: 6 additions & 1 deletion resources/styles.css
Original file line number Diff line number Diff line change
Expand Up @@ -95,10 +95,15 @@
}

.fail {
-fx-background-color: -fx-theme-fail;
-fx-background-color: -fx-theme-attention;
-fx-text-fill: #323232;
}

.attention {
-fx-background-color: -fx-theme-attention;
-fx-text-fill: #323232;
}

.tree-search {
-fx-background-insets: 1 0 1 0;
}
Expand Down
2 changes: 1 addition & 1 deletion resources/theme_dark.css
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
-fx-theme-links: #de00c0;
-fx-theme-breadcrums: #de00c0;
-fx-theme-ok: #00ff00;
-fx-theme-fail: #ff0000;
-fx-theme-attention: #ff0000;
-fx-theme-warning: orange;
-fx-theme-title-label: orange;
-fx-theme-field-label: #de00c0;
Expand Down
2 changes: 1 addition & 1 deletion resources/theme_light.css
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
-fx-theme-links: #9c0084;
-fx-theme-breadcrums: #9c0084;
-fx-theme-ok: #00ff00;
-fx-theme-fail: #ff0000;
-fx-theme-attention: #ff0000;
-fx-theme-warning: orange;
-fx-theme-title-label: #336699;
-fx-theme-field-label: #9c0084;
Expand Down
22 changes: 9 additions & 13 deletions src-dbg/flow_storm/debugger/events_processor.clj
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,7 @@
[flow-storm.debugger.ui.docs.screen :as docs-screen]
[flow-storm.debugger.ui.flows.general :as ui-general]
[flow-storm.debugger.ui.utils :as ui-utils]
[flow-storm.utils :refer [log]]
[flow-storm.debugger.state :as dbg-state :refer [dispatch-task-event]]))
[flow-storm.debugger.state :as dbg-state]))

(defn- var-instrumented-event [{:keys [var-ns var-name]}]
(ui-utils/run-later
Expand Down Expand Up @@ -47,13 +46,9 @@
(defn- task-submitted-event [_]
(ui-main/set-task-cancel-btn-enable true))

(defn- task-result-event [{:keys [task-id result]}]
(dispatch-task-event :result task-id result)
(defn- task-finished-event [_]
(ui-main/set-task-cancel-btn-enable false))

(defn- task-progress-event [{:keys [task-id progress]}]
(dispatch-task-event :progress task-id progress))

(defn- heap-info-update-event [ev-args-map]
(ui-main/update-heap-indicator ev-args-map))

Expand Down Expand Up @@ -83,9 +78,7 @@
(flows-screen/update-exceptions-combo)))

(defn process-event [[ev-type ev-args-map]]
(when (and (:debug-mode? (dbg-state/debugger-config))
(not (= ev-type :heap-info-update)))
(log (format "Processing event: %s" [ev-type ev-args-map])))

(case ev-type
:var-instrumented (var-instrumented-event ev-args-map)
:var-uninstrumented (var-uninstrumented-event ev-args-map)
Expand All @@ -94,13 +87,16 @@
:flow-created (flow-created-event ev-args-map)
:threads-updated (threads-updated-event ev-args-map)
:tap (tap-event ev-args-map)

:task-submitted (task-submitted-event ev-args-map)
:task-result (task-result-event ev-args-map)
:task-progress (task-progress-event ev-args-map)
:task-finished (task-finished-event ev-args-map)

:heap-info-update (heap-info-update-event ev-args-map)
:goto-location (goto-location-event ev-args-map)
:show-doc (show-doc-event ev-args-map)
:break-installed (break-installed-event ev-args-map)
:break-removed (break-removed-event ev-args-map)
:recording-updated (recording-updated-event ev-args-map)
:function-unwinded-event (function-unwinded-event ev-args-map)))
:function-unwinded-event (function-unwinded-event ev-args-map)
nil ;; events-processor doesn't handle every event, specially tasks processing
))
36 changes: 24 additions & 12 deletions src-dbg/flow_storm/debugger/events_queue.clj
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@
Events will be dispatched after a dispatch-fn is set with `set-dispatch-fn`"

(:require [flow-storm.state-management :refer [defstate]]
[flow-storm.utils :as utils])
[flow-storm.debugger.state :as dbg-state]
[flow-storm.utils :as utils :refer [log]])
(:import [java.util.concurrent ArrayBlockingQueue TimeUnit]))

(declare start-events-queue)
Expand All @@ -27,28 +28,39 @@
(when-let [queue (:queue events-queue)]
(.put ^ArrayBlockingQueue queue e)))

(defn set-dispatch-fn [dispatch-fn]
(reset! (:dispatch-fn events-queue) dispatch-fn))
(defn add-dispatch-fn [fn-key dispatch-fn]
(swap! (:dispatch-fns events-queue) assoc fn-key dispatch-fn))

(defn rm-dispatch-fn [fn-key]
(swap! (:dispatch-fns events-queue) dissoc fn-key))

(defn start-events-queue []
(let [events-queue (ArrayBlockingQueue. 1000)
dispatch-fn (atom nil)
dispatch-fns (atom {})
dispatch-thread (Thread.
(fn []
(try
;; don't do anything until we have a dispatch-fn
(while (and (not (.isInterrupted (Thread/currentThread)))
(not @dispatch-fn))
(utils/log "Waiting for dispatch-fn before dispatching events")
(empty? @dispatch-fns))
(utils/log "Waiting for a dispatch-fn before dispatching events")
(Thread/sleep wait-for-system-started-interval))

;; start the dispatch loop
(let [dispatch @dispatch-fn]
(loop [ev nil]
(when-not (.isInterrupted (Thread/currentThread))
(loop [ev nil]
(when-not (.isInterrupted (Thread/currentThread))
(try
(when ev
(dispatch ev))
(recur (.poll events-queue queue-poll-interval TimeUnit/MILLISECONDS)))))

(when (and (:debug-mode? (dbg-state/debugger-config))
(not (= (first ev) :heap-info-update)))
(log (format "Processing event: %s" ev)))

(doseq [dispatch (vals @dispatch-fns)]
(dispatch ev)))
(catch Exception e
(utils/log-error (str "Error dispatching event" ev) e)))
(recur (.poll events-queue queue-poll-interval TimeUnit/MILLISECONDS))))
(catch java.lang.InterruptedException _
(utils/log "Events thread interrupted"))
(catch Exception e
Expand All @@ -59,7 +71,7 @@
(.start dispatch-thread)
{:interrupt-fn interrupt-fn
:queue events-queue
:dispatch-fn dispatch-fn}))
:dispatch-fns dispatch-fns}))

(defn stop-events-queue []
(when-let [stop-fn (:interrupt-fn events-queue)]
Expand Down
2 changes: 1 addition & 1 deletion src-dbg/flow_storm/debugger/main.clj
Original file line number Diff line number Diff line change
Expand Up @@ -142,4 +142,4 @@

;; we set the events dispatch-fn afater `state-management/start` returns because
;; we know the UI is ready to start processing events
(events-queue/set-dispatch-fn events-processor/process-event))
(events-queue/add-dispatch-fn :events-processor events-processor/process-event))
59 changes: 32 additions & 27 deletions src-dbg/flow_storm/debugger/runtime_api.clj
Original file line number Diff line number Diff line change
Expand Up @@ -57,11 +57,15 @@
(callstack-node-childs [_ node])
(callstack-node-frame [_ node])
(fn-call-stats [_ flow-id thread-id])
(collect-fn-frames [_ flow-id thread-id fn-ns fn-name form-id render-args])
(find-expr-entry [_ criteria])
(total-order-timeline [_])
(thread-prints [_ print-cfg])
(async-search-next-timeline-entry [_ flow-id thread-id query-str from-idx opts])

(collect-fn-frames-task [_ flow-id thread-id fn-ns fn-name form-id render-args])
(interrupt-all-tasks [_])
(start-task [_ task-id])

(find-expr-entry-task [_ criteria])
(total-order-timeline-task [_])
(thread-prints-task [_ print-cfg])
(search-next-timeline-entry-task [_ flow-id thread-id query-str from-idx opts])
(discard-flow [_ flow-id])

(def-value [_ var-symb val-ref])
Expand All @@ -78,7 +82,6 @@

(eval-form [_ form-str opts])

(interrupt-all-tasks [_])
(clear-recordings [_])
(clear-api-cache [_])
(all-flows-threads [_])
Expand All @@ -93,7 +96,7 @@
(toggle-recording [_])
(set-total-order-recording [_ x])
(all-fn-call-stats [_])
(find-fn-call [_ fq-fn-call-symb from-idx opts]))
(find-fn-call-task [_ fq-fn-call-symb from-idx opts]))

(defn cached-apply [cache cache-key f args]
(let [res (get @cache cache-key :flow-storm/cache-miss)]
Expand Down Expand Up @@ -156,11 +159,15 @@
(callstack-node-childs [_ node] (api-call :local "callstack-node-childs" [node]))
(callstack-node-frame [_ node] (api-call :local "callstack-node-frame" [node]))
(fn-call-stats [_ flow-id thread-id] (api-call :local "fn-call-stats" [flow-id thread-id]))
(collect-fn-frames [_ flow-id thread-id fn-ns fn-name form-id render-args] (api-call :local "collect-fn-frames" [flow-id thread-id fn-ns fn-name form-id render-args]))
(find-expr-entry [_ criteria] (api-call :local "find-expr-entry" [criteria]))
(total-order-timeline [_] (api-call :local "total-order-timeline" []))
(thread-prints [_ print-cfg] (api-call :local "thread-prints" [print-cfg]))
(async-search-next-timeline-entry [_ flow-id thread-id query-str from-idx opts] (api-call :local "async-search-next-timeline-entry" [flow-id thread-id query-str from-idx opts]))

(collect-fn-frames-task [_ flow-id thread-id fn-ns fn-name form-id render-args] (api-call :local "collect-fn-frames-task" [flow-id thread-id fn-ns fn-name form-id render-args]))
(start-task [_ task-id] (api-call :local "start-task" [task-id]))
(interrupt-all-tasks [_] (api-call :local "interrupt-all-tasks" []))

(find-expr-entry-task [_ criteria] (api-call :local "find-expr-entry-task" [criteria]))
(total-order-timeline-task [_] (api-call :local "total-order-timeline-task" []))
(thread-prints-task [_ print-cfg] (api-call :local "thread-prints-task" [print-cfg]))
(search-next-timeline-entry-task [_ flow-id thread-id query-str from-idx opts] (api-call :local "search-next-timeline-entry-task" [flow-id thread-id query-str from-idx opts]))
(discard-flow [_ flow-id] (api-call :local "discard-flow" [flow-id]))
(def-value [_ var-symb val-ref] (api-call :local "def-value" [(or (namespace var-symb) "user") (name var-symb) val-ref]))
(tap-value [_ vref] (api-call :local "tap-value" [vref]))
Expand Down Expand Up @@ -217,9 +224,6 @@
;; so when re-evaluating a var (probably a function) store and restore its meta
(when v (reset-meta! v vmeta))))))))))

(interrupt-all-tasks [_]
(api-call :local "interrupt-all-tasks" []))

(clear-recordings [_]
(api-call :local "clear-recordings" []))

Expand Down Expand Up @@ -256,8 +260,8 @@
(all-fn-call-stats [_]
(api-call :local "all-fn-call-stats" []))

(find-fn-call [_ fq-fn-call-symb from-idx opts]
(api-call :local "find-fn-call" [fq-fn-call-symb from-idx opts])))
(find-fn-call-task [_ fq-fn-call-symb from-idx opts]
(api-call :local "find-fn-call-task" [fq-fn-call-symb from-idx opts])))

;;;;;;;;;;;;;;;;;;;;;;
;; For Clojure repl ;;
Expand All @@ -279,11 +283,15 @@
(callstack-node-childs [_ node] (api-call :remote "callstack-node-childs" [node]))
(callstack-node-frame [_ node] (api-call :remote "callstack-node-frame" [node]))
(fn-call-stats [_ flow-id thread-id] (api-call :remote "fn-call-stats" [flow-id thread-id]))
(collect-fn-frames [_ flow-id thread-id fn-ns fn-name form-id render-args] (api-call :remote "collect-fn-frames" [flow-id thread-id fn-ns fn-name form-id render-args]))
(find-expr-entry [_ criteria] (api-call :remote "find-expr-entry" [criteria]))
(total-order-timeline [_] (api-call :remote "total-order-timeline" []))
(thread-prints [_ print-cfg] (api-call :remote "thread-prints" [print-cfg]))
(async-search-next-timeline-entry [_ flow-id thread-id query-str from-idx opts] (api-call :remote "async-search-next-timeline-entry" [flow-id thread-id query-str from-idx opts]))

(collect-fn-frames-task [_ flow-id thread-id fn-ns fn-name form-id render-args] (api-call :remote "collect-fn-frames-task" [flow-id thread-id fn-ns fn-name form-id render-args]))
(start-task [_ task-id] (api-call :remote "start-task" [task-id]))
(interrupt-all-tasks [_] (api-call :remote "interrupt-all-tasks" []))

(find-expr-entry-task [_ criteria] (api-call :remote "find-expr-entry-task" [criteria]))
(total-order-timeline-task [_] (api-call :remote "total-order-timeline-task" []))
(thread-prints-task [_ print-cfg] (api-call :remote "thread-prints-task" [print-cfg]))
(search-next-timeline-entry-task [_ flow-id thread-id query-str from-idx opts] (api-call :remote "search-next-timeline-entry-task" [flow-id thread-id query-str from-idx opts]))
(discard-flow [_ flow-id] (api-call :remote "discard-flow" [flow-id]))
(def-value [_ var-symb val-ref]
(case (dbg-state/env-kind)
Expand Down Expand Up @@ -352,9 +360,6 @@
(safe-eval-code-str (format "(alter-meta! #'%s/%s merge %s)" ns var-name (pr-str var-meta))))
expr-res))

(interrupt-all-tasks [_]
(api-call :remote "interrupt-all-tasks" []))

(clear-recordings [_]
(api-call :remote "clear-recordings" []))

Expand Down Expand Up @@ -393,8 +398,8 @@
(all-fn-call-stats [_]
(api-call :remote "all-fn-call-stats" []))

(find-fn-call [_ fq-fn-call-symb from-idx opts]
(api-call :remote "find-fn-call" [fq-fn-call-symb from-idx opts]))
(find-fn-call-task [_ fq-fn-call-symb from-idx opts]
(api-call :remote "find-fn-call-task" [fq-fn-call-symb from-idx opts]))

Closeable
(close [_] (stop-repl))
Expand Down
11 changes: 0 additions & 11 deletions src-dbg/flow_storm/debugger/state.clj
Original file line number Diff line number Diff line change
Expand Up @@ -474,17 +474,6 @@
[]
(get-in @state [:bookmarks])))

;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
;; Pending tasks sub-system ;;
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;

(defn subscribe-to-task-event [event-key task-id callback]
(swap! state assoc-in [:pending-tasks-subscriptions [event-key task-id]] callback))

(defn dispatch-task-event [event-key task-id data]
(when-let [cb (get-in @state [:pending-tasks-subscriptions [event-key task-id]])]
(cb data)))

;;;;;;;;;;;;;;;;;;;;;;;;;;;;
;; Navigation undo system ;;
;;;;;;;;;;;;;;;;;;;;;;;;;;;;
Expand Down
Loading

0 comments on commit b34891e

Please sign in to comment.