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 <steve...@gmail.com> 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
>
> On Tue, May 19, 2015 at 4:38 AM, Amith George <strider...@gmail.com> wrote:
>> Hi,
>>
>> Thanks for taking the time to profile the code. I implemented the two
>> suggestions (using the two argument arity of lte and using aset instead of
>> aset-long).
>>
>> https://github.com/amithgeorge/reddit-dailyprogrammer-clojure/blob/2655a83f7fcf51e4fedae164d7d17386a0c8854f/src/rdp/214_intermediate_arr.clj
>>
>> lein run -m rdp.214-intermediate-arr 1 true
>> ;; took around 250s.
>>
>> The changes didn't seem to make a difference. The before and after runs all
>> take between 250 - 260s. I kept the reduce as-is. From your reply, it looks
>> like these two changes reduced the execution time by almost 30s. Any
>> thoughts of why there isn't much of a difference for me? - I am using
>> Clojure 1.7.3-beta3 and Oracle Java 1.8.0_45 on OSX Mavericks.
>>
>> On Saturday, 16 May 2015 08:32:12 UTC+5:30, Steven Yi wrote:
>>>
>>> Ah, I see.  Well, I think then you can ignore the stuff about warming
>>> up, as this certainly takes a while to run here:
>>>
>>> "Elapsed time: 314763.666693 msecs"
>>>
>>> I tried profiling with Yourkit and saw a couple of things to change:
>>>
>>> ;; I think lte with more than two args ends up being slower than
>>> unrolling out here
>>> ;; I also tried type-hinting values from paper to ^long to avoid lte
>>> calls with Number
>>> (defn- covered?
>>>   [[^long canvas-x ^long canvas-y] paper]
>>>   (and (<= ^long (:x1 paper) canvas-x ) (<= canvas-x ^long (:x2 paper))
>>>        (<= ^long (:y1 paper) canvas-y ) (<= canvas-y ^long (:y2 paper))))
>>>
>>> ;; for the reduce function in visible-color-frequencies-arr
>>> ;; using aset instead of aset-long, as it looked like aset-long was
>>> using reflection
>>>          (aset colorCounts color (+ 1 (aget colorCounts color)))
>>>
>>> That got it down to:
>>>
>>> "Elapsed time: 279864.041477 msecs"
>>>
>>> I suspect you might get improvement too if you change
>>> visible-color-frequencies-arr to use loop-recur instead of reduce
>>> since you're doing a bit of magic there.
>>>
>>> Unfortunately I have to stop at the moment as I have to leave on a
>>> trip early in the morning, but hopefully that's useful.
>>>
>>> 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 a topic in the
>> Google Groups "Clojure" group.
>> To unsubscribe from this topic, visit
>> https://groups.google.com/d/topic/clojure/JgxFQLP2E34/unsubscribe.
>> To unsubscribe from this group and all its topics, send an email to
>> clojure+unsubscr...@googlegroups.com.
>> For more options, visit https://groups.google.com/d/optout.

-- 
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.

Reply via email to