Clojure

n-ary bit functions, also inlining of n-ary bit and math operations

Details

  • Type: Enhancement Enhancement
  • Status: Closed Closed
  • Priority: Major Major
  • Resolution: Completed
  • Affects Version/s: None
  • Fix Version/s: Release 1.3
  • Component/s: None
  • Labels:
    None
  • Approval:
    Ok

Description

Add an &more version to the bit operations bit-and bit-or bit-xor bit-and-not (via reduce),
Inline the n-ary version of the math ops, + - * /, inline the n-ary versions of bit-and bit-or bit-xor bit-and-not

  1. 184-inlining-nary-math.diff
    30/Apr/11 3:59 PM
    6 kB
    Alan Dipert
  2. 184-inlining-nary-math-2.diff
    06/May/11 12:39 PM
    5 kB
    Alan Dipert
  3. 184-inlining-nary-math-3.diff
    09/May/11 11:42 PM
    5 kB
    Alan Dipert
  4. inlinemath.patch
    22/Apr/11 11:40 AM
    4 kB
    Christopher Redinger

Activity

Hide
Assembla Importer added a comment -

Jonathan A Smith said: [file:dlFRFQKPyr3RaoeJe5aVNr]: inline math and bit-or patch file

Show
Assembla Importer added a comment - Jonathan A Smith said: [file:dlFRFQKPyr3RaoeJe5aVNr]: inline math and bit-or patch file
Hide
Assembla Importer added a comment -

Jonathan A Smith said: original math defs stay, new ones get re-defined after proxy is available.

Show
Assembla Importer added a comment - Jonathan A Smith said: original math defs stay, new ones get re-defined after proxy is available.
Hide
Assembla Importer added a comment -

mikehinchey said: Applies clean (though has extra whitespace and tabs). Existing tests still pass.

Show
Assembla Importer added a comment - mikehinchey said: Applies clean (though has extra whitespace and tabs). Existing tests still pass.
Hide
Assembla Importer added a comment -

Jonathan A Smith said: The defn functions are within a let, which means that they get tabbed in by Emacs ctrl+alt+Q. Is this the referred to 'extra whitespace and tabs'?

I think that this line:
>0-arities (rule-set (fn [x] (> x 1)))

might be a mistake, and perhaps should be:
>0-arities (rule-set (fn [x] (> x 0)))

But I will have to look at it and rebuild it when I get home tonight. resulting tests would still pass as the only effected function would be the single arity subtraction operation.

Show
Assembla Importer added a comment - Jonathan A Smith said: The defn functions are within a let, which means that they get tabbed in by Emacs ctrl+alt+Q. Is this the referred to 'extra whitespace and tabs'? I think that this line: >0-arities (rule-set (fn [x] (> x 1))) might be a mistake, and perhaps should be: >0-arities (rule-set (fn [x] (> x 0))) But I will have to look at it and rebuild it when I get home tonight. resulting tests would still pass as the only effected function would be the single arity subtraction operation.
Hide
Assembla Importer added a comment -

mikehinchey said: "git am" complained about whitespace at the ends of lines. When I tab on each line to re-indent, emacs replaced tabs with spaces and deleted space at the end. Also, you could combine the 2 let blocks into 1.

Show
Assembla Importer added a comment - mikehinchey said: "git am" complained about whitespace at the ends of lines. When I tab on each line to re-indent, emacs replaced tabs with spaces and deleted space at the end. Also, you could combine the 2 let blocks into 1.
Hide
Assembla Importer added a comment -

richhickey said: This still needs work. I'd like to avoid the need for proxy. It would be easy to make Compiler.isInline() in the compiler presume :inline-arities is an IFn (so sets literals would still work, but so would predicate fns)

Show
Assembla Importer added a comment - richhickey said: This still needs work. I'd like to avoid the need for proxy. It would be easy to make Compiler.isInline() in the compiler presume :inline-arities is an IFn (so sets literals would still work, but so would predicate fns)
Hide
Assembla Importer added a comment -

richhickey said: I've made that change, so isInline now treats :inline-arities as a predicate IFn. This should allow for a simpler implementation

Show
Assembla Importer added a comment - richhickey said: I've made that change, so isInline now treats :inline-arities as a predicate IFn. This should allow for a simpler implementation
Rich Hickey made changes -
Field Original Value New Value
Fix Version/s Release.Next [ 10038 ]
Fix Version/s Approved Backlog [ 10034 ]
Reporter Assembla Importer [ importer ]
Priority Blocker [ 1 ]
Christopher Redinger made changes -
Priority Blocker [ 1 ] Major [ 3 ]
Christopher Redinger made changes -
Assignee Alan Dipert [ alan@thinkrelevance.com ]
Hide
Christopher Redinger added a comment -

Adding patch from Assembla

Show
Christopher Redinger added a comment - Adding patch from Assembla
Christopher Redinger made changes -
Attachment inlinemath.patch [ 10196 ]
Hide
Alan Dipert added a comment -

inlining and n-ary bit functions and math ops

  • n-ary versions and inlines of bit-and, bit-or, bit-xor, bit-and-not
  • n-ary inlines for +, +', *, *', /, -, -'

This patch doesn't optimize / such that the & more version expands to equivalent of (/ x (reduce * y more)) rather than (reduce / (/ x y) more). The original patch had a divide-by-zero bug, and I think adding this should be another ticket.

Show
Alan Dipert added a comment - inlining and n-ary bit functions and math ops
  • n-ary versions and inlines of bit-and, bit-or, bit-xor, bit-and-not
  • n-ary inlines for +, +', *, *', /, -, -'
This patch doesn't optimize / such that the & more version expands to equivalent of (/ x (reduce * y more)) rather than (reduce / (/ x y) more). The original patch had a divide-by-zero bug, and I think adding this should be another ticket.
Alan Dipert made changes -
Attachment 184-inlining-nary-math.diff [ 10214 ]
Alan Dipert made changes -
Description Add an &more version to the bit operations bit-and bit-or bit-xor bit-and-not (via reduce),
Inline the n-ary version of the math ops, + - * /, inline the n-ary versions of bit-and bit-or bit-xor bit-and-not
Optimization to / such that the & more version expands to equivalent of (/ x (reduce * y more)) rather than (reduce / (/ x y) more)
Add an &more version to the bit operations bit-and bit-or bit-xor bit-and-not (via reduce),
Inline the n-ary version of the math ops, + - * /, inline the n-ary versions of bit-and bit-or bit-xor bit-and-not
Hide
Alan Dipert added a comment -

Created CLJ-785 for optimizing n-ary /

Show
Alan Dipert added a comment - Created CLJ-785 for optimizing n-ary /
Alan Dipert made changes -
Waiting On stu
Hide
Alexander Taggart added a comment -

You'd have less copypasta using this:

(defn ^:private binary-inline
  ([op]
    (fn
      ([x y] `(. clojure.lang.Numbers (~op ~x ~y)))
      ([x y & more]
        (reduce1
          (fn [a b] `(. clojure.lang.Numbers (~op ~a ~b)))
          `(. clojure.lang.Numbers (~op ~x ~y)) more))))
  ([op unchecked-op]
    (if *unchecked-math*
      (binary-inline unchecked-op)
      (binary-inline op))))

(defn ^:private >1? [n] (clojure.lang.Numbers/gt n 1))

Also, ^:static doesn't do anything anymore.

The above was from the branch I made, that I mentioned in the thread on ticket status. It also included a fix for the inefficient divide.

Show
Alexander Taggart added a comment - You'd have less copypasta using this:
(defn ^:private binary-inline
  ([op]
    (fn
      ([x y] `(. clojure.lang.Numbers (~op ~x ~y)))
      ([x y & more]
        (reduce1
          (fn [a b] `(. clojure.lang.Numbers (~op ~a ~b)))
          `(. clojure.lang.Numbers (~op ~x ~y)) more))))
  ([op unchecked-op]
    (if *unchecked-math*
      (binary-inline unchecked-op)
      (binary-inline op))))

(defn ^:private >1? [n] (clojure.lang.Numbers/gt n 1))
Also, ^:static doesn't do anything anymore. The above was from the branch I made, that I mentioned in the thread on ticket status. It also included a fix for the inefficient divide.
Hide
Alan Dipert added a comment -

Thanks, I'll extract a patch for this from your branch.

Show
Alan Dipert added a comment - Thanks, I'll extract a patch for this from your branch.
Alan Dipert made changes -
Waiting On stu
Hide
Alan Dipert added a comment -

This patch incorporates Alex Taggart's work in the comments, but leaves out the change to /. It made a call to the nonexistent Numbers/unchecked_divide method and multiplies in the denominator, which is faster but has different overflow behavior than divide/reduce.

Show
Alan Dipert added a comment - This patch incorporates Alex Taggart's work in the comments, but leaves out the change to /. It made a call to the nonexistent Numbers/unchecked_divide method and multiplies in the denominator, which is faster but has different overflow behavior than divide/reduce.
Alan Dipert made changes -
Attachment 184-inlining-nary-math-2.diff [ 10218 ]
Alan Dipert made changes -
Waiting On stu
Hide
Alexander Taggart added a comment -

One problem with the patch: while the bit-ops fns have n-ary support for their inlines, the fns themselves do not. They need to be updated to something like:

(defn bit-and
  "Bitwise and"
  {:inline (nary-inline 'and)
   :inline-arities >1?
   :added "1.0"}
  ([x y] (. clojure.lang.Numbers and x y))
  ([x y & more]
    (reduce1 bit-and (bit-and x y) more)))
Show
Alexander Taggart added a comment - One problem with the patch: while the bit-ops fns have n-ary support for their inlines, the fns themselves do not. They need to be updated to something like:
(defn bit-and
  "Bitwise and"
  {:inline (nary-inline 'and)
   :inline-arities >1?
   :added "1.0"}
  ([x y] (. clojure.lang.Numbers and x y))
  ([x y & more]
    (reduce1 bit-and (bit-and x y) more)))
Alan Dipert made changes -
Waiting On stu
Hide
Alan Dipert added a comment - - edited

Attached 184-inlining-nary-math-3.diff. Same as previous patch, but adds n-ary version of bit fns in addition to inlines, which were missing as Alex Taggart pointed out. Thanks for catching that!

Show
Alan Dipert added a comment - - edited Attached 184-inlining-nary-math-3.diff. Same as previous patch, but adds n-ary version of bit fns in addition to inlines, which were missing as Alex Taggart pointed out. Thanks for catching that!
Alan Dipert made changes -
Attachment 184-inlining-nary-math-3.diff [ 10220 ]
Hide
Alan Dipert added a comment -

Patch is in 1.3-alpha7

Show
Alan Dipert added a comment - Patch is in 1.3-alpha7
Alan Dipert made changes -
Status In Progress [ 3 ] Resolved [ 5 ]
Resolution Completed [ 1 ]
Aaron Bedra made changes -
Approval Test Ok
Stuart Halloway made changes -
Status Resolved [ 5 ] Closed [ 6 ]

People

Vote (0)
Watch (0)

Dates

  • Created:
    Updated:
    Resolved: