thanks, I had a vague memory there was an escape hatch, but could not remember

no go. Does it matter what order the readers are installed?

I added debug breakpts in pathom, and only saw lots of calls to nested resolution..never saw that part of the program ever get the top-level list…

Map reader’s where I found that ::p/final, and when I watch what it does, I only ever see the individual nested entities. It never sees the list, so never skips it

Yeah, I’m getting into here in connect:

(defn- process-simple-reader-response [{:keys [query] :as env} response]
  (let [key (-> env :ast :key)
        x   (get response key)]
    (cond
      (and query (sequential? x))
      (->> (mapv atom x) (p/join-seq env))

      (nil? x)
      (if (contains? response key)
        nil
        ::p/continue)

      :else
      (p/join (atom x) env))))

key is :products/all-products, and x is my list of 10,000 things, but it ignore metadata here and just processes them all anyway…unless, the query is nil

the thing that takes care of it is the map-reader

https://github.com/wilkerlucio/pathom/blob/214834442615352df15c854e44d537bf2ced96a2/src/com/wsscode/pathom/core.cljc#L758

I’m tracing code with breakpoints

that is where I land, and then it does all 10000

I can't find process-simple-reader-response in pathom, isn't this in your codebase?

connect.cljc

v 2.2.30

ah, right

did you tried bumping to 2.3.0? I think that fn got removed

no, but 2.3.0 was waaaaay slower

can’t risk that in prod

I guess I could try and diagnose that, but I’ve already spent 5 hours on what should have been easy optimizations 😄

ok, let me check on older version to understand

I’ll try 2.3 and see if there was something weird

ok, weird

pathom 2.3 seems faster

@wilkerlucio

let me eval this

Yeah, I got one call this time

cool

I wonder if it was slower earlire for some other reason

I’ll measure some more. Thanks for the help 🙂

if that gets to a hard problem I can release a hotfix for 2.3.x with a fix for that fn you pointed out

I didn’t have the metadata on there earlier

so, I do think 2.3 is possibly slower without it. Let me check that

yeah, if you learn what it is, I would be glad to address, I don't remember any big change that could cause noticeable performance degradation

yeah…it’s waaaaaay worse

but maybe was this change, from moving to inline re-processing to the reader engine

I was getting 8s

it’s been running for 50s so far

and I was getting 2.5s with 2.2.30 with the metadata

still running!

q: :taxable/taxes                     536   504.60μs    30.54ms    54.41ms    57.27ms    60.14ms    65.60ms    30.09ms  ±50%    16.13s     71%
q: :product/tax                       535   188.04μs    11.61ms    21.51ms    22.96ms    25.35ms    29.50ms    12.01ms  ±51%     6.43s     28%
q: :entity/company                    536    34.74μs    52.49μs    72.69μs    96.27μs   128.20μs   359.06μs    56.05μs  ±23%    30.04ms     0%
q: :tax/category                      535    18.01μs    35.83μs    45.29μs    55.18μs    85.55μs   214.91μs    36.65μs  ±23%    19.61ms     0%
q: :product/precise-price             535    14.47μs    24.50μs    38.12μs    42.49μs    86.47μs   414.02μs    28.26μs  ±31%    15.12ms     0%
q: :tax/rate                          535    13.50μs    20.57μs    38.35μs    44.99μs    71.57μs   106.56μs    25.30μs  ±35%    13.53ms     0%
q: :product/inventory-tracked?        536    12.16μs    20.13μs    36.27μs    41.60μs    77.72μs   103.24μs    23.69μs  ±35%    12.70ms     0%
q: :product/description               535    12.71μs    19.32μs    35.91μs    38.43μs    74.16μs   102.37μs    23.24μs  ±35%    12.43ms     0%
q: :product/id                        536    11.15μs    18.36μs    35.35μs    41.61μs    76.47μs   127.77μs    22.69μs  ±38%    12.16ms     0%
q: :product/sku                       535    11.39μs    19.33μs    35.35μs    38.69μs    77.51μs   104.34μs    22.38μs  ±36%    11.98ms     0%

I have minute-by-minute timing in a plugin

measuring every call to the reader

It can normally do 10,000 of those in a few ms…but with 2.3 it is 10x that

or more

I’m seeing 11ms for 500 calls

I think the reason may be on the switch from inline processing to reader engine, but I wouldn't expect to be that much

still running

I think it may be 100 to 1000x slower

yeah, I def cannot risk that one in prod 😕

this would kill us

if you need I can do a hotfix for that 2.2 to add support for final there

yes please 🙂

repro case is easy

(defresolver all-products-resolver
  [{:keys [conn] :as env} _]
  {::pc/output [{:product/all-products [:product/id
                                        :product/sku
                                        :product/description
                                        :product/precise-price
                                        :taxable/taxes
                                        :product/tax
                                        :product/inventory-tracked?]}]}
  (let [db         (d/db conn)
        company-id (env->effective-company-id env)]
    {:product/all-products (get-all-products db company-id)}))

and make get-all-products return 1000 (complete) things

please try 2.2.31-SNAPSHOT

k

thanks

(it’s still running btw)

this is running on CLJ, right?

yeah

because I wanna do some measurements later, k

8700k CPU

8700k?

Intel CPU model

not the latest, but no slouch

ah, ok, just wanted to make sure I'm doing the tests in the same env (clj vs cljs)

sure

please let me know if that works, if so I'll release 2.2.31

I had to restart, and this thing is rather large…takes a min

no worries and no rush 🙂

cognitect.transit/write              transit.clj:  167
                 com.cognitect.transit.impl.WriterFactory$1.write       WriterFactory.java:   62
                      com.cognitect.transit.impl.JsonEmitter.emit         JsonEmitter.java:   41
            com.cognitect.transit.impl.AbstractEmitter.marshalTop     AbstractEmitter.java:  211
               com.cognitect.transit.impl.AbstractEmitter.marshal     AbstractEmitter.java:  184
               com.cognitect.transit.impl.AbstractEmitter.emitMap     AbstractEmitter.java:   85
                   com.cognitect.transit.impl.JsonEmitter.emitMap         JsonEmitter.java:  171
               com.cognitect.transit.impl.AbstractEmitter.marshal     AbstractEmitter.java:  194
       java.lang.Exception: Not supported: class clojure.lang.Atom
java.lang.RuntimeException: java.lang.Exception: Not supported: class clojure.lang.Atom

ooops

atom flowed through

ah, I know what is

sorry, minor mistake, releasing again

I should have cloned and run local/root so I could pull and jsut refresh instead of restarting 😕

re-released

https://github.com/wilkerlucio/pathom/tree/hotfix-final-2.3

k

its on this branch if you wanna pull local

did u push?

yes

k…just a min

arg, it trigerred reformat, messy diff, going to fix, but the code should be correct

clean diff now: https://github.com/wilkerlucio/pathom/commit/2b1a4c368d9fdb170b31f28531608ad92d1eecce

that got it

fast

network request now 800ms

including data transfer

cool

I'm doing a few changes because I'm afraid the current impl may try to call meta in values it shouldn't

ok, I’m running local/root so much faster to test now

oh, I think its fine, I though (meta nil) would break, but it works

unless you find some issue, I'm ok to release the 2.2.31

your branch name is weird

yeah... its wrong

so as long as the code is right, it should be ok

I mean version *

its fine, I'll just remove it after

and has the tag to get back to this version if needed

hm

hold on

I clicked around a bit

and other stuff seems to be slower now

any idea why that might be?

basically it looks like similar queries that don’t have the metadata are slower now

by a noticeable amount

are you making a bunch of new atoms to fix this?

or rather, without the metadata would it have new overhead?

nope, it should be simple

I just made another version, with an even simpler change

try the current on that branch

this are all the changes; https://github.com/wilkerlucio/pathom/compare/hotfix-final-2.3?expand=1

the previous was a bit weird: https://github.com/wilkerlucio/pathom/commit/2b1a4c368d9fdb170b31f28531608ad92d1eecce

ok, yeah, looks good. I bounced back to .31 and that page was just as slow, so false alarm

hey, I just did a comparison between 2.2 and 2.3 processing a 10k size list

(ns com.wsscode.demos.perf
  (:require [com.wsscode.pathom.connect :as pc]
            [criterium.core :as c]
            [com.wsscode.pathom.core :as p]))

(pc/defresolver long-list [env _]
  {::pc/output [:items]}
  {:items (mapv #(hash-map :id 1) (range 10000))})

(pc/defresolver x [env {:keys [id]}]
  {::pc/input #{:id}
   ::pc/output [:x]}
  {:x (* 10 id)})

(def registry [long-list x])

(def parser
  (p/parser
    {::p/env     {::p/reader               [p/map-reader
                                            pc/reader2
                                            pc/open-ident-reader
                                            p/env-placeholder-reader]
                  ::p/placeholder-prefixes #{">"}}
     ::p/mutate  pc/mutate
     ::p/plugins [(pc/connect-plugin {::pc/register registry})
                  p/error-handler-plugin
                  p/trace-plugin]}))

(comment
  (c/with-progress-reporting
    (c/quick-bench
      (parser {} [{:items [:x]}]))))

Pathom 2.2: Execution time mean : 93.091257 µs

Pathom 2.3: Execution time mean : 97.180679 µs

so aparently its not just something about the reader process, I wonder if there is something around custom plugins or anything that got affected to make such difference in your case

I think you need each item to have props

mine has 3 levels of depth, but you’re right, it could be a plugin

I did not actually measure real overhead

(of every bit)

should final work ok with siblings?

ah, I think I had an error on the measure, got redo

what you mean siblings?

query [{:a […]} {:other […]}]..can both siblings say final?

on their lists of return values

i.e. resolver for :a and :other

yeah, you can make any list or a map final

ok, yeah, I think that is working…I’ve peppered it around and broke something, so was verifying I understood

@wilkerlucio could you push that to clojars? (at least the snapshot)?

I want to try on my staging server, and it won’t have local root

no problem, you mean at this commit: https://github.com/wilkerlucio/pathom/compare/hotfix-final-2.3?expand=1

correct?

the last one you did

yeah

I’m at 58b3d629e6ef464199a06d7fcde627c403fa9d59

pushed snapshot for you to try on staging

Did you see phronmophobic on #fulcro getting Fulcro working with Swing (I think…maybe AWT) 🙂

thanks

entertaining

wow, didn't, that's cool!

(snapshot ends in -3, if you need to confirm during some pipeline)

yeah, not retrieving yet…clojars must be slow

duh…should just use a frikkin sha

This seems to be a problem in dev now. If I reload all nses for dev work, it pulls in pathom, which pulls in gen, which unfortunately pulls in Fulcro 2 stuff:

#error{:cause "Could not locate fulcro/client/primitives__init.class, fulcro/client/primitives.clj or fulcro/client/primitives.cljc on classpath.",
       :via [{:type clojure.lang.Compiler$CompilerException,
              :message "Syntax error compiling at (com/wsscode/pathom/gen.cljc:1:1).",
              :data #:clojure.error{:phase :compile-syntax-check,
                                    :line 1,
                                    :column 1,
                                    :source "com/wsscode/pathom/gen.cljc"},
              :at [clojure.lang.Compiler load "Compiler.java" 7647]}
             {:type java.io.FileNotFoundException,
              :message "Could not locate fulcro/client/primitives__init.class, fulcro/client/primitives.clj or fulcro/client/primitives.cljc on classpath.",

but you didn’t change anything in this rev…

hm, perhaps I’ll try 2.3.0 w/guardrails disabled 🙂