From 0c7aced207a4bd0bfec8c5d5c2f4352f9d1be201 Mon Sep 17 00:00:00 2001 From: Daniel Compton Date: Fri, 9 Feb 2018 05:36:55 +1300 Subject: [PATCH] Use all subscription values between each epoch for calculating sub state Using filtered-traces means that you will lose sub-state updates. The filtered-traces are partial parts of an epoch, but it is only passed to sub-state matching once there is a full epoch matched. At that point filtered-epochs will not necessarily have a matching complement of traces. This commit uses all of the stored traces on each run, meaning that no traces will be dropped. --- src/day8/re_frame/trace/events.cljs | 7 +- src/day8/re_frame/trace/metamorphic.cljc | 58 ++++++------ src/day8/re_frame/trace/view/debug.cljs | 7 +- src/day8/re_frame/trace/view/traces.cljs | 116 ++++++++++++----------- 4 files changed, 100 insertions(+), 88 deletions(-) diff --git a/src/day8/re_frame/trace/events.cljs b/src/day8/re_frame/trace/events.cljs index 57c0357..96bc8d5 100644 --- a/src/day8/re_frame/trace/events.cljs +++ b/src/day8/re_frame/trace/events.cljs @@ -366,7 +366,6 @@ :trace-panel/update-show-epoch-traces? [(rf/path [:trace-panel :show-epoch-traces?]) (fixed-after #(localstorage/save! "show-epoch-traces?" %))] (fn [_ [k show-epoch-traces?]] - (js/console.log k show-epoch-traces?) show-epoch-traces?)) ;; App DB @@ -523,6 +522,8 @@ {drop-re-frame :re-frame drop-reagent :reagent} (get-in db [:settings :low-level-trace]) all-traces (reduce conj previous-traces filtered-traces) parse-state (metam/parse-traces parse-state filtered-traces) + ;; TODO:!!!!!!!!!!!!! We should be parsing everything else with the traces that span the newly matched + ;; epochs, not the filtered-traces, as these are only partial. new-matches (:partitions parse-state) previous-matches (get-in db [:epochs :matches] []) parse-state (assoc parse-state :partitions []) ;; Remove matches we know about @@ -536,10 +537,12 @@ ;; like its reagent id, when it was created, run, disposed, what values it returned, e.t.c. subscription-info (metam/subscription-info (get-in db [:epochs :subscription-info] {}) filtered-traces (get-in db [:app-db :reagent-id])) sub-state (get-in db [:epochs :sub-state] metam/initial-sub-state) - subscription-match-state (metam/subscription-match-state sub-state filtered-traces new-matches) + subscription-match-state (metam/subscription-match-state sub-state all-traces new-matches) subscription-matches (rest subscription-match-state) + new-sub-state (last subscription-match-state) timing (mapv (fn [match] + ;; TODO: this isn't quite correct, shouldn't be using filtered-traces here (let [epoch-traces (into [] (comp (utils/id-between-xf (:id (first match)) (:id (last match)))) diff --git a/src/day8/re_frame/trace/metamorphic.cljc b/src/day8/re_frame/trace/metamorphic.cljc index 925ae51..eee18f5 100644 --- a/src/day8/re_frame/trace/metamorphic.cljc +++ b/src/day8/re_frame/trace/metamorphic.cljc @@ -296,38 +296,38 @@ (defn process-sub-traces [initial-state traces] - (let [first-pass (reduce (fn [state trace] - (let [tags (get trace :tags) - reaction-id (:reaction tags) - state (-> state - (update-in [reaction-id :order] (fnil conj []) (:op-type trace)) - ;; In a perfect world we could provide this only in the :sub/create branch, but we have - ;; zombie reactions roaming the DOM, so we re-add it on every trace in case a sub was - ;; disposed of previously (and removed from the sub state). - (assoc-in [reaction-id :subscription] (:query-v tags))) - new-state - (case (:op-type trace) - :sub/create (-> state - (assoc-in [reaction-id :created?] true) - (assoc-in [reaction-id :subscription] (:query-v tags))) - :sub/run (update state reaction-id (fn [sub-state] - ;; TODO: should we keep track of subscriptions that have been disposed - ;; so we can detect zombies? + (let [first-pass (reduce (fn [init-state trace] + (let [tags (get trace :tags) + reaction-id (:reaction tags) + state (-> init-state + (update-in [reaction-id :order] (fnil conj []) (:op-type trace)) + ;; In a perfect world we could provide this only in the :sub/create branch, but we have + ;; zombie reactions roaming the DOM, so we re-add it on every trace in case a sub was + ;; disposed of previously (and removed from the sub state). + (assoc-in [reaction-id :subscription] (:query-v tags))) + new-state + (case (:op-type trace) + :sub/create (-> state + (assoc-in [reaction-id :created?] true) + (assoc-in [reaction-id :subscription] (:query-v tags))) + :sub/run (update state reaction-id (fn [sub-state] + ;; TODO: should we keep track of subscriptions that have been disposed + ;; so we can detect zombies? - ;; TODO: this should only update once per phase, even if a sub runs multiple times - (-> (if (contains? sub-state :value) - (assoc sub-state :previous-value (:value sub-state)) - sub-state) - (assoc :run? true - :value (:value tags))))) - :sub/dispose (assoc-in state [reaction-id :disposed?] true))] - (when-not (contains? (get new-state reaction-id) :subscription) - #?(:cljs (js/console.warn trace (get new-state reaction-id)))) + ;; TODO: this should only update once per phase, even if a sub runs multiple times + (-> (if (contains? sub-state :value) + (assoc sub-state :previous-value (:value sub-state)) + sub-state) + (assoc :run? true + :value (:value tags))))) + :sub/dispose (assoc-in state [reaction-id :disposed?] true))] + (when-not (contains? (get new-state reaction-id) :subscription) + #?(:cljs (js/console.warn trace (get new-state reaction-id)))) - new-state)) - initial-state - traces) + new-state)) + initial-state + traces) second-pass (reduce (fn [all-state [sub-id sub-state]] ;; TODO: integrate this into the first pass for efficiency (if (and (contains? sub-state :previous-value) diff --git a/src/day8/re_frame/trace/view/debug.cljs b/src/day8/re_frame/trace/view/debug.cljs index 1fdddf6..46d2615 100644 --- a/src/day8/re_frame/trace/view/debug.cljs +++ b/src/day8/re_frame/trace/view/debug.cljs @@ -16,7 +16,12 @@ [rc/label :label (str "Current epoch ID " (prn-str @(rf/subscribe [:epochs/current-epoch-id])))] [:h2 "Subscriptions"] - [components/simple-render @(rf/subscribe [:subs/sub-state]) ["debug-subs"]] + [components/simple-render @(rf/subscribe [:subs/current-epoch-sub-state]) ["debug-subs"]] + [:h2 "pre epoch"] + [components/simple-render @(rf/subscribe [:subs/inter-epoch-subs]) ["pre-epoch-subs"]] + [:h2 "match state"] + [components/simple-render @(rf/subscribe [:epochs/current-match-state]) ["match-state"]] + [rc/label :label "Epochs"] (let [current-match @(rf/subscribe [:epochs/current-match])] diff --git a/src/day8/re_frame/trace/view/traces.cljs b/src/day8/re_frame/trace/view/traces.cljs index 556eac8..c7b9433 100644 --- a/src/day8/re_frame/trace/view/traces.cljs +++ b/src/day8/re_frame/trace/view/traces.cljs @@ -18,63 +18,67 @@ (rf/dispatch [:traces/add-filter filter-input filter-type])) (defn render-traces [visible-traces filter-items filter-input trace-detail-expansions] - (doall - (->> - visible-traces - (map-indexed (fn [index {:keys [op-type id operation tags duration] :as trace}] - (let [show-row? (get-in @trace-detail-expansions [:overrides id] - (:show-all? @trace-detail-expansions)) - op-name (if (vector? operation) - (second operation) - operation) - #_#__ (js/console.log (devtools/header-api-call tags))] - (list [:tr {:key id - :on-click #(rf/dispatch [:traces/toggle-trace id]) - :class (str/join " " ["trace--trace" - (case op-type - :sub/create "trace--sub-create" - :sub/run "trace--sub-run" - :sub/dispose "trace--sub-run" - :event "trace--event" - :render "trace--render" - :re-frame.router/fsm-trigger "trace--fsm-trigger" - nil)])} + (let [debug? @(rf/subscribe [:settings/debug?])] + (doall + (->> + visible-traces + (map-indexed (fn [index {:keys [op-type id operation tags duration] :as trace}] + (let [show-row? (get-in @trace-detail-expansions [:overrides id] + (:show-all? @trace-detail-expansions)) + op-name (if (vector? operation) + (second operation) + operation) + #_#__ (js/console.log (devtools/header-api-call tags))] + (list [:tr {:key id + :on-click #(rf/dispatch [:traces/toggle-trace id]) + :class (str/join " " ["trace--trace" + (case op-type + :sub/create "trace--sub-create" + :sub/run "trace--sub-run" + :sub/dispose "trace--sub-run" + :event "trace--event" + :render "trace--render" + :re-frame.router/fsm-trigger "trace--fsm-trigger" + nil)])} + [:td.trace--toggle + [:button.expansion-button (if show-row? "▼" "▶")]] + [:td.trace--op + [:span.op-string {:on-click (fn [ev] + (add-filter filter-items (name op-type) :contains) + (.stopPropagation ev))} + (str op-type)]] + [:td.trace--op-string + [:span.op-string {:on-click (fn [ev] + (add-filter filter-items (name op-name) :contains) + (.stopPropagation ev))} + (pp/truncate 20 :middle (pp/str->namespaced-sym op-name)) " " + [:span + {:style {:opacity 0.5 + :display "inline-block"}} + (when-let [[_ & params] (or (get tags :query-v) + (get tags :event))] + (->> (map pp/pretty-condensed params) + (str/join ", ") + (pp/truncate-string :middle 40)))]]] + (if debug? + [:td.trace--meta + (:reaction (:tags trace)) "/" id] + [:td.trace--meta - [:td.trace--toggle - [:button.expansion-button (if show-row? "▼" "▶")]] - [:td.trace--op - [:span.op-string {:on-click (fn [ev] - (add-filter filter-items (name op-type) :contains) - (.stopPropagation ev))} - (str op-type)]] - [:td.trace--op-string - [:span.op-string {:on-click (fn [ev] - (add-filter filter-items (name op-name) :contains) - (.stopPropagation ev))} - (pp/truncate 20 :middle (pp/str->namespaced-sym op-name)) " " - [:span - {:style {:opacity 0.5 - :display "inline-block"}} - (when-let [[_ & params] (or (get tags :query-v) - (get tags :event))] - (->> (map pp/pretty-condensed params) - (str/join ", ") - (pp/truncate-string :middle 40)))]]] - [:td.trace--meta - (.toFixed duration 1) " ms"]] - (when show-row? - [:tr.trace--details {:key (str id "-details") - :tab-index 0} - [:td] - [:td.trace--details-tags {:col-span 2 - :on-click #(.log js/console trace)} - [:div.trace--details-tags-text - (let [tag-str (prn-str tags)] - (str (subs tag-str 0 400) - (when (< 400 (count tag-str)) - " ...")))]] - [:td.trace--meta.trace--details-icon - {:on-click #(.log js/console tags)}]])))))))) + (.toFixed duration 1) " ms"])] + (when show-row? + [:tr.trace--details {:key (str id "-details") + :tab-index 0} + [:td] + [:td.trace--details-tags {:col-span 2 + :on-click #(.log js/console trace)} + [:div.trace--details-tags-text + (let [tag-str (prn-str tags)] + (str (subs tag-str 0 400) + (when (< 400 (count tag-str)) + " ...")))]] + [:td.trace--meta.trace--details-icon + {:on-click #(.log js/console tags)}]]))))))))) (defn render [] (let [filter-input (r/atom "")