Destructuring and keyword access

[Christophe Grand]

Hi,

Currently associative destructuring always expands to calls to get even when the key is a keyword.
I have a small patch which make it expands to direct keyword lookups.
May I open an issue for that?

Thanks,

Christophe

[Tassilo Horn]

Christophe Grand <chris...@cgrand.net> writes:

Hi Christophe,

> Currently associative destructuring always expands to calls to get
> even when the key is a keyword. I have a small patch which make it
> expands to direct keyword lookups.

What's the benefit of that? At least a quick benchmark seems to
indicate that the performance is equal.

--8<---------------cut here---------------start------------->8---
user=> (use 'criterium.core)
nil
user=> (let [m {:a 1, :b 2, :c 3, :d 4, :e 5, :f 6, :g 7, :h 8}]
(bench (get m :g))
(bench (:g m)))
Evaluation count : 173624160 in 60 samples of 2893736 calls.
Execution time mean : 353.140625 ns
Execution time std-deviation : 26.118695 ns
Execution time lower quantile : 339.868576 ns ( 2.5%)
Execution time upper quantile : 374.317787 ns (97.5%)

Found 4 outliers in 60 samples (6.6667 %)
low-severe 2 (3.3333 %)
low-mild 2 (3.3333 %)
Variance from outliers : 55.1380 % Variance is severely inflated by outliers
Evaluation count : 166152240 in 60 samples of 2769204 calls.
Execution time mean : 368.768440 ns
Execution time std-deviation : 14.631760 ns
Execution time lower quantile : 357.451880 ns ( 2.5%)
Execution time upper quantile : 398.427724 ns (97.5%)

Found 2 outliers in 60 samples (3.3333 %)
low-severe 2 (3.3333 %)
Variance from outliers : 25.4904 % Variance is moderately inflated by outliers
nil
--8<---------------cut here---------------end--------------->8---

Bye,
Tassilo

[Christophe Grand]

Direct keyword lookups are optimized call sites for types implementing IKeywordLookup (eg records).

=> (use 'criterium.core)
nil
=> (defrecord R [a b c d e f g h])
bench.core.R
=> (let [m (R. 1 2 3 4 5 6 7 8)]

     (bench (get m :g))
     (bench (:g m)))

Evaluation count : 330878040 in 60 samples of 5514634 calls.
             Execution time mean : 188,486839 ns
    Execution time std-deviation : 12,514961 ns
   Execution time lower quantile : 179,788178 ns ( 2,5%)
   Execution time upper quantile : 220,171461 ns (97,5%)

Found 6 outliers in 60 samples (10,0000 %)
    low-severe     2 (3,3333 %)
    low-mild     4 (6,6667 %)
 Variance from outliers : 50,0898 % Variance is severely inflated by outliers
Evaluation count : 503316480 in 60 samples of 8388608 calls.
             Execution time mean : 121,512183 ns
    Execution time std-deviation : 13,544785 ns
   Execution time lower quantile : 116,452694 ns ( 2,5%)
   Execution time upper quantile : 130,043954 ns (97,5%)

Found 8 outliers in 60 samples (13,3333 %)
    low-severe     4 (6,6667 %)
    low-mild     4 (6,6667 %)
 Variance from outliers : 73,8211 % Variance is severely inflated by outliers
nil

Christophe

--
You received this message because you are subscribed to the Google Groups "Clojure Dev" group.
To post to this group, send email to cloju...@googlegroups.com.
To unsubscribe from this group, send email to clojure-dev...@googlegroups.com.
For more options, visit this group at http://groups.google.com/group/clojure-dev?hl=en.

--
Professional: http://cgrand.net/ (fr)
On Clojure: http://clj-me.cgrand.net/ (en)

[Tassilo Horn]

Christophe Grand <chris...@cgrand.net> writes:

> Direct keyword lookups are optimized call sites for types implementing
> IKeywordLookup (eg records).

Ah, ok, that makes a lot of sense.

Bye,
Tassilo

[Christophe Grand]

On a related subject: may I open an issue to have PersistentHashMap to implement IKeywordLookup? It proves to be more efficient (nearly halves the lookup time)  than I would have thought.

Christophe

[Brandon Bloom]

> Direct keyword lookups are optimized call sites for types implementing IKeywordLookup (eg records).

Wouldn't it be preferable to enhance the call optimization, rather than the destructuring code generation?

If you make this enhancement to the call site optimizer, then the improvement is automatically available to any other macros that generate associative lookups. Consider pattern matching, unification, and other value extracting forms.

[Christophe Grand]

You are right: in addition to the proposed change to destructure, the inline expansion of get should be modified to emit direct keyword lookups when possible.

Christophe

[Brandon Bloom]

Why "in addition to" vs "instead of"? If 'get expands inline to a direct keyword lookup on (keyword? key), then there is no reason to bypass 'get during destructuring.

[Rich Hickey]

Yes, thanks.

[Rich Hickey]

On Oct 23, 2012, at 10:08 AM, Christophe Grand wrote:

> On a related subject: may I open an issue to have PersistentHashMap to implement IKeywordLookup? It proves to be more efficient (nearly halves the lookup time) than I would have thought.
>

Do you have a gist or something outlining the approach? What you are saying here is not enough to go on.

Thanks,

Rich

[Christophe Grand]

Thanks, done: http://dev.clojure.org/jira/browse/CLJ-1096

--
You received this message because you are subscribed to the Google Groups "Clojure Dev" group.
To post to this group, send email to cloju...@googlegroups.com.
To unsubscribe from this group, send email to clojure-dev...@googlegroups.com.
For more options, visit this group at http://groups.google.com/group/clojure-dev?hl=en.

[Christophe Grand]

Sure, so, here is a patch: https://github.com/cgrand/clojure/commit/8c483f76cf7abb26db15fd7fb814de79f4e4e5f6

And some quick measures:

=> (let [m (reduce #(assoc % (keyword (str %2)) %2) {} "abcdefghijklmnopqr")]
    (time   
      (dotimes [_ 1e8] (:k m)))
    (time
      (dotimes [_ 1e8] (get m :k))))
"Elapsed time: 5838.101 msecs"
"Elapsed time: 16338.974 msecs"
nil

As I said I didn't expected that much of a boost.

Christophe