Add unchecked-divide, unchecked-remainder

Question

This appears like it might be an oversight that these are missing. There are unchecked-divide-int and unchecked-remainder-int functions, but not equivalents for longs, even though there are equivalents for longs for every other unchecked operation. The JVM has bytecodes for long division and remainder.

The Clojure documentation in the section "Support for Java Primitives" on page http://clojure.org/java_interop has links for unchecked-divide and unchecked-remainder, but since they don't exist in Clojure, the API link targets don't exist.

It seems like a good idea to either add these to Clojure, or remove them from the documentation.

Answer 1

Comment made by: coltnz

Having a go at this.

Answer 2

Comment made by: coltnz

• Added tests for unchecked-divide-int and unchecked-remainder-int too.
• Unchecked fns only support binary arity and will throw CompilerException(ArityException)s where checked will not.
• Is there any value to (int,long) (long,int) overrides for java interop cases e.g. using java collections from Clojure in high perf code?

Answer 3

Comment made by: alexmiller

Thanks for taking this on Colin!

1) When I apply the patch (git apply CLJ-1545.diff), I get a bunch of whitespace errors which will need to be cleaned up but also the patch seems to fail to apply at all on the changes in test/clojure/test_clojure/numbers.clj. It looks like perhaps the diff is just not the right diff format. You might want to check out the instructions at http://dev.clojure.org/display/community/Developing Patches about using git format-patch.

2) If you could put a more useful git commit message, that would be helpful. Something like "CLJ-1545 Adds missing unchecked-divide and unchecked-remainder for primitive longs."

Thanks!

Answer 4

Comment made by: coltnz

Uggh, sorry Alex.

New patch with better commit message.

Answer 5

Comment made by: alexmiller

The patch format looks better. Pulling out farther to the ticket itself, afaict Clojure will already use the right byteocode for checked or unchecked so this may not even be needed?

If I compile (without the patch):

(defn foo-div ^long [^long a ^long b] (quot a b))

then the bytecode for that fn is:

`
public final long invokePrim(long, long);

Code:
   0: lload_1
   1: lload_3
   2: ldiv
   3: lreturn

`

similarly, quot of two longs yields the same code but with lrem. I think patch has no net effect on the resulting bytecode?

Answer 6

Comment made by: jafingerhut

Alex, did you do the testing in your previous comment with **unchecked-math** true or false? If false, then I would think that if CLJ-1254 is judged a bug, then the behavior you saw is a bug, too, that misses the same corner case.

Answer 7

Comment made by: alexmiller

The current results are the same with either unchecked-math setting, but I see your point.

Refreshing my memory of the {{(/ Long/MIN_VALUE -1)}} case, I think you're right. The (new) unchecked-divide / remainder should do what the current (checked) forms do and the regular division and remainder cases should be making the overflow check. I think CLJ-1254 should cover the quot changes.

Answer 8

Comment made by: coltnz

user=> (dotimes (link: 6) (time (dotimes (link: 50000000) (unchecked-divide 4 (System/currentTimeMillis)))))
"Elapsed time: 1806.942 msecs"
"Elapsed time: 1808.747 msecs"
"Elapsed time: 1865.074 msecs"
"Elapsed time: 1802.777 msecs"
"Elapsed time: 1839.468 msecs"
"Elapsed time: 1830.61 msecs"
nil
user=> (dotimes (link: 6) (time (dotimes (link: 50000000) (/ 4 (System/currentTimeMillis)))))
"Elapsed time: 5003.598 msecs"
"Elapsed time: 4998.182 msecs"
"Elapsed time: 4941.237 msecs"
"Elapsed time: 5036.517 msecs"
"Elapsed time: 4965.867 msecs"
"Elapsed time: 4982.693 msecs"

Answer 9

Reference: https://clojure.atlassian.net/browse/CLJ-1545 (reported by jafingerhut)