Wow. This is rather hard to believe. But the execution time is now 10s. :D All of your suggestions were on the mark. I implemented them in stages/commits. The command to execute the file remains the same - `lein run -m rdp.214-intermediate-arr 1 true`
1. Replaced the coordinates vector with separate x and y. Added relevant type hints. https://github.com/amithgeorge/reddit-dailyprogrammer-clojure/blob/0ba1992ff892928beb71400fb44cce471318d187/src/rdp/214_intermediate_arr.clj#L60 Best time - 110s. Can't believe that simply changing from `[[^long x ^long y]]` to `[^long x ^long y]` can reduce the execution time by half!! 2. Replaced the reduce in visible-color-frequencies-arr to nested loop-recurs. https://github.com/amithgeorge/reddit-dailyprogrammer-clojure/blob/75db0f839b748a4ca313bc319c4970a9a0329fb6/src/rdp/214_intermediate_arr.clj#L86 Best time - 90s. I thought reduce was equivalent to a loop recur. Might have to create a separate bare-minimum code sample to investigate why reduce was slower. 3. Stored papers in a native array. The parsed input still contains the papers in a vector. Its only when the main function is called with array true, that we convert to an array and store as a local value. https://github.com/amithgeorge/reddit-dailyprogrammer-clojure/blob/0f49b7979f9c2e0e7a522f5ad875de0807d4e2da/src/rdp/214_intermediate_arr.clj#L115 Best time - 10s. I really don't get why the `some` over a vector was so slow. I tried replacing the `some` with a loop-recur (one style using an incrementing index and `get papers index` and the other style using `rest papers`.) Surprisingly both styles were slower than using `some`. Thanks a lot for figuring out the slow areas. You made me aware of two tools I could use to find bottlenecks. At the very least, I now know Clojure doesn't necessarily mean 10x slower. That said, I really wanna know why storing papers in a native array made such a difference. Ideally I shouldn't have to expose a mutable array to other functions. In this case, the functions that accept/use the mutable array are meant to be private and under my control, so I could have some measure of confidence that mutation wouldn't occur. On Tuesday, 19 May 2015 23:02:11 UTC+5:30, Steven Yi wrote: > > Hi Amith, > > One last optimization yields a 4x increase over the last version, and > now 12x over the original: > > "Elapsed time: 22848.384642 msecs" > > Code here: > > https://gist.github.com/kunstmusik/db6e14118e818abf3bb8 > > Changes from last version: > > - parse-inputs - Put parsed Paper objects into an array instead of a > vector. > - visible-color iterates through the array of Paper objects using a > loop-recur > > I spent a moment looking at this using JVisualVM and saw from the > previous version that there was a lot of time spent in sequence > related stuff and in the some function. I realized from using some it > was going through the vector items pretty often, and since that list > of items is static, are read-only within the context of the code, and > the values aren't shared around a codebase, I went ahead and optimized > it to use a fixed array. > > Hope this runs well on your side too! > steven > > > On Tue, May 19, 2015 at 12:49 PM, Steven Yi <stev...@gmail.com > <javascript:>> wrote: > > Hi Amith, > > > > Just got back from a trip and had a chance to look at this again. I'm > > not sure why you didn't see a change there; I'm running the same > > version of Clojure and Java here. > > > > I did another change to move from using a reduce to nested > > loop-recurs. That with a some additional type hints for ^Paper, and > > moving the retrieval of :height and :width outside of the loops got a > > performance increase of almost 3x from the original: > > > > "Elapsed time: 109335.307414 msecs" > > > > The modified code is here: > > > > https://gist.github.com/kunstmusik/e1081d417142a90d5cfa > > > > Some general notes: > > > > - covered? - type-hinted ^Paper > > - visible-color - switched to pass in ^long x and y, used fn form for > > passed-in function and type-hinted ^Paper argument > > - visible-color-frequencies - switched to nested loop-recur, moved out > > height and width to single call outside critical loop > > > > Could you try the version from the gist there to see if you get a > > similar speedup? > > > > Cheers! > > steven > > > -- You received this message because you are subscribed to the Google Groups "Clojure" group. To post to this group, send email to clojure@googlegroups.com Note that posts from new members are moderated - please be patient with your first post. To unsubscribe from this group, send email to clojure+unsubscr...@googlegroups.com For more options, visit this group at http://groups.google.com/group/clojure?hl=en --- You received this message because you are subscribed to the Google Groups "Clojure" group. To unsubscribe from this group and stop receiving emails from it, send an email to clojure+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
