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?
> 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---
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
On Tue, Oct 23, 2012 at 1:45 PM, Tassilo Horn <t...@gnu.org> wrote:
> Christophe Grand <christo...@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.
> 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
> --
> You received this message because you are subscribed to the Google Groups
> "Clojure Dev" group.
> To post to this group, send email to clojure-dev@googlegroups.com.
> To unsubscribe from this group, send email to
> clojure-dev+unsubscribe@googlegroups.com.
> For more options, visit this group at
> http://groups.google.com/group/clojure-dev?hl=en.
Christophe Grand <christo...@cgrand.net> writes:
> Direct keyword lookups are optimized call sites for types implementing
> IKeywordLookup (eg records).
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
On Tue, Oct 23, 2012 at 11:18 AM, Christophe Grand <christo...@cgrand.net>wrote:
> 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?
> 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.
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
On Tue, Oct 23, 2012 at 8:27 PM, Brandon Bloom <snprbo...@gmail.com> wrote:
> > 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.
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.
On Wednesday, October 24, 2012 7:03:21 AM UTC-7, Christophe Grand wrote:
> 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
> On Tue, Oct 23, 2012 at 8:27 PM, Brandon Bloom <snpr...@gmail.com<javascript:> > > wrote:
>> > 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.
On Oct 23, 2012, at 5:18 AM, Christophe Grand wrote:
> 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?
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.
On Mon, Oct 29, 2012 at 12:58 PM, Rich Hickey <richhic...@gmail.com> wrote:
> On Oct 23, 2012, at 5:18 AM, Christophe Grand wrote:
> > 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?
> Yes, thanks.
> --
> You received this message because you are subscribed to the Google Groups
> "Clojure Dev" group.
> To post to this group, send email to clojure-dev@googlegroups.com.
> To unsubscribe from this group, send email to
> clojure-dev+unsubscribe@googlegroups.com.
> For more options, visit this group at
> http://groups.google.com/group/clojure-dev?hl=en.
On Mon, Oct 29, 2012 at 1:03 PM, Rich Hickey <richhic...@gmail.com> wrote:
> 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.