Clojure

GC Issue 95: max-key and min-key evaluate k multiple times for arguments

Details

  • Type: Enhancement Enhancement
  • Status: Open Open
  • Priority: Minor Minor
  • Resolution: Unresolved
  • Affects Version/s: None
  • Fix Version/s: None
  • Component/s: None
  • Labels:
  • Patch:
    Code and Test
  • Approval:
    Triaged

Description

max-key or min-key will evaluate (k value) multiple times for arguments if more than 2 arguments are passed. This is undesirable if k is expensive to calculate.

Good perf test:

(apply max-key #(Math/tan %) (range 1000))

Approach: Avoid duplication evaluation of k for any element.

Patch: clj-99-v2.patch

  1. clj-99-v2.patch
    09/Feb/15 2:50 PM
    3 kB
    Michael Blume
  2. clj-99-v1.diff
    22/Oct/13 7:54 PM
    3 kB
    Andy Fingerhut

Activity

Hide
Assembla Importer added a comment -
Show
Assembla Importer added a comment - Converted from http://www.assembla.com/spaces/clojure/tickets/99
Hide
Assembla Importer added a comment -

richhickey said: Updating tickets (#8, #19, #30, #31, #126, #17, #42, #47, #50, #61, #64, #69, #71, #77, #79, #84, #87, #89, #96, #99, #103, #107, #112, #113, #114, #115, #118, #119, #121, #122, #124)

Show
Assembla Importer added a comment - richhickey said: Updating tickets (#8, #19, #30, #31, #126, #17, #42, #47, #50, #61, #64, #69, #71, #77, #79, #84, #87, #89, #96, #99, #103, #107, #112, #113, #114, #115, #118, #119, #121, #122, #124)
Hide
Andy Fingerhut added a comment -

clj-99-min-key-max-key-performance-v1.txt dated Nov 15 2012 changes min-key and max-key to evaluate the function k on each of its other arguments at most once.

Show
Andy Fingerhut added a comment - clj-99-min-key-max-key-performance-v1.txt dated Nov 15 2012 changes min-key and max-key to evaluate the function k on each of its other arguments at most once.
Hide
Andy Fingerhut added a comment -

Patch clj-99-v1.diff is same as previously attached patch, but with .diff suffix for easier diff-style viewing in some editors.

Show
Andy Fingerhut added a comment - Patch clj-99-v1.diff is same as previously attached patch, but with .diff suffix for easier diff-style viewing in some editors.
Hide
Steve Kim added a comment -

sort-by is similarly inefficient. It calls keyfn for every comparison in sort, not once for every element to be sorted.

I'm not familiar with this project's preferred workflow, so I have to ask: Do you want me to open a separate ticket for sort-by, or do you prefer to consolidate that issue into this ticket?

Show
Steve Kim added a comment - sort-by is similarly inefficient. It calls keyfn for every comparison in sort, not once for every element to be sorted. I'm not familiar with this project's preferred workflow, so I have to ask: Do you want me to open a separate ticket for sort-by, or do you prefer to consolidate that issue into this ticket?
Hide
Alex Miller added a comment -

Separate.

Show
Alex Miller added a comment - Separate.
Hide
Steve Kim added a comment -

I created CLJ-1402 for sort-by

Show
Steve Kim added a comment - I created CLJ-1402 for sort-by
Hide
Michael Blume added a comment -

clj-99-v2 applies cleanly to master

Show
Michael Blume added a comment - clj-99-v2 applies cleanly to master

People

Vote (3)
Watch (3)

Dates

  • Created:
    Updated: