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.

Sign up for GitHub

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

Jump to bottom

Failing to compile with shadow-cljs "release" #5

Closed
alex314159 opened this issue Feb 3, 2023 · 18 comments
Closed

Failing to compile with shadow-cljs "release" #5

alex314159 opened this issue Feb 3, 2023 · 18 comments

Comments

@alex314159
Copy link

Hello Chris,

Using shadow-cljs latest, I can work with tmdjs in a dev environment, but release fails with the following:

PS C:\Users\AAlmosni\IdeaProjects\gui2023> lein prod
[:app] Compiling ...
------ WARNING #1 -  -----------------------------------------------------------
 Resource: ham_fisted/BitmapTrie.js:213:5
 variable i is undeclared
--------------------------------------------------------------------------------
------ WARNING #2 -  -----------------------------------------------------------
 Resource: ham_fisted/BitmapTrie.js:477:8
 variable LFPprops is undeclared
--------------------------------------------------------------------------------
------ WARNING #3 -  -----------------------------------------------------------
 Resource: ham_fisted/BitmapTrie.js:542:22
 variable get is undeclared
--------------------------------------------------------------------------------
------ WARNING #4 -  -----------------------------------------------------------
 Resource: ham_fisted/BitmapTrie.js:611:68
 variable bfn is undeclared
--------------------------------------------------------------------------------
------ WARNING #5 -  -----------------------------------------------------------
 Resource: ham_fisted/BitmapTrie.js:774:53
 variable curEntry is undeclared
--------------------------------------------------------------------------------
------ WARNING #6 -  -----------------------------------------------------------
 Resource: ham_fisted/BitmapTrie.js:864:5
 variable n is undeclared
--------------------------------------------------------------------------------
------ WARNING #7 -  -----------------------------------------------------------
 Resource: ham_fisted/BitmapTrie.js:924:15
 variable put is undeclared
--------------------------------------------------------------------------------
------ WARNING #8 -  -----------------------------------------------------------
 Resource: ham_fisted/BitmapTrie.js:934:20
 variable containsKey is undeclared
--------------------------------------------------------------------------------
------ WARNING #9 -  -----------------------------------------------------------
 Resource: ham_fisted/BitmapTrie.js:935:23
 variable remove is undeclared
--------------------------------------------------------------------------------
------ WARNING #10 -  ----------------------------------------------------------
 Resource: ham_fisted/BitmapTrie.js:945:11
 variable f is undeclared
--------------------------------------------------------------------------------
------ WARNING #11 -  ----------------------------------------------------------
 Resource: ham_fisted/BitmapTrie.js:1059:24
 variable nullEntry is undeclared
--------------------------------------------------------------------------------
------ WARNING #12 -  ----------------------------------------------------------
 Resource: ham_fisted/BitmapTrie.js:1382:51
 variable defaultHashProvider is undeclared
--------------------------------------------------------------------------------
------ WARNING #13 -  ----------------------------------------------------------
 Resource: ham_fisted/ChunkedVec.js:50:10
 variable shallowClone is undeclared
--------------------------------------------------------------------------------
------ WARNING #14 -  ----------------------------------------------------------
 Resource: ham_fisted/ChunkedVec.js:54:20
 variable setOwner is undeclared
--------------------------------------------------------------------------------
------ WARNING #15 -  ----------------------------------------------------------
 Resource: ham_fisted/ChunkedVec.js:146:1
 variable mutAssoc is undeclared
--------------------------------------------------------------------------------
------ WARNING #16 -  ----------------------------------------------------------
 Resource: ham_fisted/ChunkedVec.js:194:12
 variable ChunkedList is undeclared
--------------------------------------------------------------------------------
nil
Closure compilation failed with 2 errors
--- ham_fisted/BitmapTrie.js:518
Class or object literal contains duplicate member "clone". In non-strict code, the last duplicate will overwrite the others.
--- ham_fisted/BitmapTrie.js:542
Class or object literal contains duplicate member "nth". In non-strict code, the last duplicate will overwrite the others.

Error encountered performing task 'run' with profile(s): 'prod'
Suppressed exit

This is my project.clj

:aliases {"dev"  ["with-profile" "dev" "run" "-m" "shadow.cljs.devtools.cli" "watch" "app"]
            "prod" ["with-profile" "prod" "run" "-m" "shadow.cljs.devtools.cli" "release" "app"]}

and shadow-cljs.edn

:builds {:app {:target          :browser
                :output-dir      "resources/public/js/compiled"
                :asset-path      "/js/compiled"
                :modules         {:app {:init-fn gui2023.core/init
                                        ;:preloads [devtools.preload]
                                        }}
                :devtools        {:http-root    "resources/public"
                                  :http-port    8280
                                  :preloads []              ;day8.re-frame-10x.preload
                                  }
                :dev
                {:compiler-options
                 {:closure-defines
                  {                                         ;re-frame.trace.trace-enabled?        true
                   ;day8.re-frame.tracing.trace-enabled? true
                   }}}
                :release
                {:build-options
                 {:ns-aliases
                  {}}}                                      ;day8.re-frame.tracing day8.re-frame.tracing-stubs
                }}
 }

Any idea what needs to be done? thanks,
@cnuernber
Copy link
Owner

I think I need to fix those issues - they are simple ones to fix. The performance benefits of ham-scripted are 10X+ the base clojurescript hashmap so it is well worth it especially when the datasets grow.

@cnuernber
Copy link
Owner

cnuernber commented Feb 3, 2023 •
edited
Loading

I think these are fixed. I cleaned up the errors and warnings and the minimal tests pass in dev compilation mode. If your app doesn't work as expected in release I think it will be because of missing symbols - something didn't get marked as an extern - and I will revisit a bit more thoroughly with the testapp in tmdjs.

2.000-beta-3

@alex314159
Copy link
Author

That worked :)) thank you!

@alex314159
Copy link
Author

I spoke too early I'm afraid. Thanks to your changes the app compiles, however the production js fails with a cryptic JS message (as all the code is compiled) when a (ds/->dataset ...) function is called. Very annoying as dev works perfectly - is there a way I can help narrow down the problem? Using node 19 and shadow 2.18.

Thank you,

@cnuernber
Copy link
Owner

What is the message?

@cnuernber
Copy link
Owner

Those really can be tough to debug - I wonder what the best way to debug it is. Perhaps instrumenting some of the code with println will but that will require I think customizing tmdjs.

@alex314159
Copy link
Author

Thanks Chris. The console says:

2app.js:5031 Uncaught TypeError: c is not a function
    at XDa.reduce (app.js:5777:24)
    at h.xa (app.js:5798:34)
    at oh (app.js:4199:62)
    at Tf (app.js:4201:52)
    at h.xa (app.js:4573:29)
    at qd.Da (app.js:4120:427)
    at qd (app.js:4120:191)
    at Object.reduce (app.js:5770:205)
    at qDa (app.js:5741:447)
    at XDa.addAll (app.js:5776:49)

The addAll error is

  qDa(null, (g,k)=>{
                        g.add(k);
                        return g

The reduce error is:

reduce(a, b) {
            a = nDa(a);
            const c = this.kb.$j
              , d = this.length
              , e = this.data
              , f = Math.ceil(d / 32) | 0;
            if (c(b))
                return this.ll.bk(b);
            for (let g = 0; g < f; ++g) {
                const k = e[g]
                  , l = Math.min(32, d - 32 * g) | 0;
                for (let m = 0; m < l; ++m)
                    if (b = a(b, k[m]),
                    c(b))
                        return this.kb.bk(b)
            }
            return b

I will try a minimal example and see if I can learn more.

@cnuernber
Copy link
Owner

That is the chunked vector reduction code. C in this case is checking if the initial value is reduced. kb is the hash provider.

In any case this looks like a totally legit issue and something I will have to get into; it is in ham-scripted not in tmdjs specifically so perhaps ham-scripted needs release-mode tests.

@cnuernber
Copy link
Owner

And more specifically it looks like the hash provider's member variables are being 'optimized' differently across modules.

@alex314159
Copy link
Author

Thank you for investigating! I will play around with ham-scripted and see if I find out more!

@cnuernber
Copy link
Owner

The api declares the real hash provider here. I wonder if that means those declarations are not mangled while the compiler in ChunkedVec is assuming they are and then by extension I wonder if hash provider member variable references should all be quoted strings to stop the compilation.

@cnuernber cnuernber reopened this Feb 28, 2023
@cnuernber
Copy link
Owner

I updated ham-scripted and tmdjs to run unit tests both in debug mode and release mode and fixed all the fallout including exactly your issue.

2.000-beta-5 should work for you but let's keep this issue open until we know for sure.

@alex314159
Copy link
Author

this has worked Chris, legend!! Thank you!

as a small gotcha in java (ds/filter-column dataset col value) works whereas JS seems to want the more exact (ds/filter-column dataset col #(= % value))

@cnuernber
Copy link
Owner

Ah - I will get to that. The reason is for future proofing. In the event the column has an index, passing in a value or a set of values allows the index to participate in the query. Passing in an opaque function does not.

@cnuernber
Copy link
Owner

Fixed, scalars and sets are supported and tested.

@alex314159
Copy link
Author

haha nice thanks!

@cnuernber
Copy link
Owner

Speaking of filtering - since you are working with timeseries data - keep in mind that tech.v3.datatype.argops has a binary-search mechanism that will be far faster than filtering in many cases -

cljs.user> (def small (ds/->dataset {:t (range 50000 51000)}))
#'cljs.user/small
cljs.user> (def big (ds/->dataset {:t (range 51000)}))
#'cljs.user/big
cljs.user> (argops/binary-search (range 51000) 50000)
50000
cljs.user> (time (argops/binary-search (range 51000) 50000))
"Elapsed time: 0.090592 msecs"
"Elapsed time: 0.090592 msecs"
50000
cljs.user> ;;Let's take advantage of the fact the column is sorted.
cljs.user> (defn sorted-take-last
             [ds col val]
             (let [vidx (argops/binary-search (ds col) val)]
               (ds/select-rows ds (range vidx (ds/row-count ds)))))
#'cljs.user/sorted-take-last
cljs.user> (sorted-take-last big :t 50000)
#dataset[unnamed [1000 1]
|    :t |
|------:|
| 50000 |
| 50001 |
| 50002 |
| 50003 |
| 50004 |
|   ... |
| 50994 |
| 50995 |
| 50996 |
| 50997 |
| 50998 |
| 50999 |]
cljs.user> (time (sorted-take-last big :t 50000))
"Elapsed time: 0.553483 msecs"
"Elapsed time: 0.553483 msecs"
#dataset[unnamed [1000 1]
|    :t |
|------:|
| 50000 |
| 50001 |
| 50002 |
| 50003 |
| 50004 |
|   ... |
| 50994 |
| 50995 |
| 50996 |
| 50997 |
| 50998 |
| 50999 |]
cljs.user>

@alex314159
Copy link
Author

super interesting - thanks Chris!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@cnuernber @alex314159