<< Back to previous view

[CLJ-1152] PermGen leak in multimethods and protocol fns when evaled Created: 30/Jan/13  Updated: 18/Apr/14

Status: Open
Project: Clojure
Component/s: None
Affects Version/s: Release 1.4
Fix Version/s: Release 1.7

Type: Defect Priority: Critical
Reporter: Chouser Assignee: Unassigned
Resolution: Unresolved Votes: 5
Labels: memory, protocols

Approval: Vetted

 Description   

There is a PermGen memory leak that we have tracked down to protocol methods and multimethods called inside an eval, because of the caches these methods use. The problem only arises when the value being cached is an instance of a class (such as a function or reify) that was defined inside the eval. Thus extending IFn or dispatching a multimethod on an IFn are likely triggers.

My fellow LonoClouder, Jeff Dik describes how to reproduce and work around the problem:

The easiest way that I have found to test this is to set "-XX:MaxPermSize" to a reasonable value so you don't have to wait too long for the PermGen space to fill up, and to use "-XX:+TraceClassLoading" and "-XX:+TraceClassUnloading" to see the classes being loaded and unloaded.

leiningen project.clj
(defproject permgen-scratch "0.1.0-SNAPSHOT"
  :dependencies [[org.clojure/clojure "1.5.0-RC1"]]
  :jvm-opts ["-XX:MaxPermSize=32M"
             "-XX:+TraceClassLoading"
             "-XX:+TraceClassUnloading"])

You can use lein swank 45678 and connect with slime in emacs via M-x slime-connect.

To monitor the PermGen usage, you can find the Java process to watch with "jps -lmvV" and then run "jstat -gcold <PROCESS_ID> 1s". According to the jstat docs, the first column (PC) is the "Current permanent space capacity (KB)" and the second column (PU) is the "Permanent space utilization (KB)". VisualVM is also a nice tool for monitoring this.

Multimethod leak

Evaluating the following code will run a loop that eval's (take* (fn foo [])).

multimethod leak
(defmulti take* (fn [a] (type a)))

(defmethod take* clojure.lang.Fn
  [a]
  '())

(def stop (atom false))
(def sleep-duration (atom 1000))

(defn run-loop []
  (when-not @stop
    (eval '(take* (fn foo [])))
    (Thread/sleep @sleep-duration)
    (recur)))

(future (run-loop))

(reset! sleep-duration 0)

In the lein swank session, you will see many lines like below listing the classes being created and loaded.

[Loaded user$eval15802$foo__15803 from __JVM_DefineClass__]
[Loaded user$eval15802 from __JVM_DefineClass__]

These lines will stop once the PermGen space fills up.

In the jstat monitoring, you'll see the amount of used PermGen space (PU) increase to the max and stay there.

-    PC       PU        OC          OU       YGC    FGC    FGCT     GCT
 31616.0  31552.7    365952.0         0.0      4     0    0.000    0.129
 32000.0  31914.0    365952.0         0.0      4     0    0.000    0.129
 32768.0  32635.5    365952.0         0.0      4     0    0.000    0.129
 32768.0  32767.6    365952.0      1872.0      5     1    0.000    0.177
 32768.0  32108.2    291008.0     23681.8      6     2    0.827    1.006
 32768.0  32470.4    291008.0     23681.8      6     2    0.827    1.006
 32768.0  32767.2    698880.0     24013.8      8     4    1.073    1.258
 32768.0  32767.2    698880.0     24013.8      8     4    1.073    1.258
 32768.0  32767.2    698880.0     24013.8      8     4    1.073    1.258

A workaround is to run prefer-method before the PermGen space is all used up, e.g.

(prefer-method take* clojure.lang.Fn java.lang.Object)

Then, when the used PermGen space is close to the max, in the lein swank session, you will see the classes created by the eval'ing being unloaded.

[Unloading class user$eval5950$foo__5951]
[Unloading class user$eval3814]
[Unloading class user$eval2902$foo__2903]
[Unloading class user$eval13414]

In the jstat monitoring, there will be a long pause when used PermGen space stays close to the max, and then it will drop down, and start increasing again when more eval'ing occurs.

-    PC       PU        OC          OU       YGC    FGC    FGCT     GCT
 32768.0  32767.9    159680.0     24573.4      6     2    0.167    0.391
 32768.0  32767.9    159680.0     24573.4      6     2    0.167    0.391
 32768.0  17891.3    283776.0     17243.9      6     2   50.589   50.813
 32768.0  18254.2    283776.0     17243.9      6     2   50.589   50.813

The defmulti defines a cache that uses the dispatch values as keys. Each eval call in the loop defines a new foo class which is then added to the cache when take* is called, preventing the class from ever being GCed.

The prefer-method workaround works because it calls clojure.lang.MultiFn.preferMethod, which calls the private MultiFn.resetCache method, which completely empties the cache.

Protocol leak

The leak with protocol methods similarly involves a cache. You see essentially the same behavior as the multimethod leak if you run the following code using protocols.

protocol leak
(defprotocol ITake (take* [a]))

(extend-type clojure.lang.Fn
  ITake
  (take* [this] '()))

(def stop (atom false))
(def sleep-duration (atom 1000))

(defn run-loop []
  (when-not @stop
    (eval '(take* (fn foo [])))
    (Thread/sleep @sleep-duration)
    (recur)))

(future (run-loop))

(reset! sleep-duration 0)

Again, the cache is in the take* method itself, using each new foo class as a key.

A workaround is to run -reset-methods on the protocol before the PermGen space is all used up, e.g.

(-reset-methods ITake)

This works because -reset-methods replaces the cache with an empty MethodImplCache.



 Comments   
Comment by Chouser [ 30/Jan/13 9:10 AM ]

I think the most obvious solution would be to constrain the size of the cache. Adding an item to the cache is already not the fastest path, so a bit more work could be done to prevent the cache from growing indefinitely large.

That does raise the question of what criteria to use. Keep the first n entries? Keep the n most recently used (which would require bookkeeping in the fast cache-hit path)? Keep the n most recently added?

Comment by Jamie Stephens [ 18/Oct/13 9:35 AM ]

At a minimum, perhaps a switch to disable the caches – with obvious performance impact caveats.

Seems like expensive LRU logic is probably the way to go, but maybe don't have it kick in fully until some threshold is crossed.

Comment by Alex Miller [ 18/Oct/13 4:28 PM ]

A report seeing this in production from mailing list:
https://groups.google.com/forum/#!topic/clojure/_n3HipchjCc

Comment by Adrian Medina [ 10/Dec/13 11:43 AM ]

So this is why we've been running into PermGen space exceptions! This is a fairly critical bug for us - I'm making extensive use of multimethods in our codebase and this exception will creep in at runtime randomly.

Comment by Kevin Downey [ 17/Apr/14 9:52 PM ]

it might be better to split this in to two issues, because at a very abstract level the two issues are the "same", but concretely they are distinct (protocols don't really share code paths with multimethods), keeping them together in one issue seems like a recipe for a large hard to read patch





[CLJ-47] GC Issue 43: Dead code in generated bytecode Created: 17/Jun/09  Updated: 26/Aug/13

Status: Open
Project: Clojure
Component/s: None
Affects Version/s: None
Fix Version/s: Backlog

Type: Defect Priority: Major
Reporter: Anonymous Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None

Approval: Vetted

 Description   
Reported by Levente.Santha, Jan 11, 2009
The bug was described in detail in this thread: http://groups.google.com/
group/clojure/browse_thread/thread/81ba15d7e9130441

For clojure.core$last__2954.invoke the correct bytecode would be (notice 
the removed "goto    65" after "41:  goto    0"):

public java.lang.Object invoke(java.lang.Object)   throws 
java.lang.Exception;
  Code:
   0:   getstatic       #22; //Field const__0:Lclojure/lang/Var;
   3:   invokevirtual   #37; //Method clojure/lang/Var.get:()Ljava/lang/
Object;
   6:   checkcast       #39; //class clojure/lang/IFn
   9:   aload_1
   10:  invokeinterface #41,  2; //InterfaceMethod clojure/lang/IFn.invoke:
(Ljava/lang/Object;)Ljava/lang/Object;
   15:  dup
   16:  ifnull  44
   19:  getstatic       #47; //Field java/lang/Boolean.FALSE:Ljava/lang/
Boolean;
   22:  if_acmpeq       45
   25:  getstatic       #22; //Field const__0:Lclojure/lang/Var;
   28:  invokevirtual   #37; //Method clojure/lang/Var.get:()Ljava/lang/
Object;
   31:  checkcast       #39; //class clojure/lang/IFn
   34:  aload_1
   35:  invokeinterface #41,  2; //InterfaceMethod clojure/lang/IFn.invoke:
(Ljava/lang/Object;)Ljava/lang/Object;
   40:  astore_1
   41:  goto    0
   44:  pop
   45:  getstatic       #26; //Field const__1:Lclojure/lang/Var;
   48:  invokevirtual   #37; //Method clojure/lang/Var.get:()Ljava/lang/
Object;
   51:  checkcast       #39; //class clojure/lang/IFn
   54:  aload_1
   55:  aconst_null
   56:  astore_1
   57:  invokeinterface #41,  2; //InterfaceMethod clojure/lang/IFn.invoke:
(Ljava/lang/Object;)Ljava/lang/Object;
   62:  areturn

Our JIT reported incorrect stack size along the basic block introduced by 
the unneeded goto.
The bug was present in SVN rev 1205.


 Comments   
Comment by Assembla Importer [ 08/Oct/10 10:21 AM ]

Converted from http://www.assembla.com/spaces/clojure/tickets/47

Comment by Assembla Importer [ 08/Oct/10 10:21 AM ]

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)

Comment by Assembla Importer [ 08/Oct/10 10:21 AM ]

aredington said: This appears to still be a problem with the generated bytecode in 1.3.0. Examining the bytecode for last, the problem has moved to invokeStatic:

<pre>
public static java.lang.Object invokeStatic(java.lang.Object) throws java.lang.Exception;
Code:
0: aload_0
1: invokestatic #50; //Method clojure/core$next.invokeStatic:(Ljava/lang/Object;)Ljava/lang/Object;
4: dup
5: ifnull 25
8: getstatic #56; //Field java/lang/Boolean.FALSE:Ljava/lang/Boolean;
11: if_acmpeq 26
14: aload_0
15: invokestatic #50; //Method clojure/core$next.invokeStatic:(Ljava/lang/Object;)Ljava/lang/Object;
18: astore_0
19: goto 0
22: goto 30
25: pop
26: aload_0
27: invokestatic #59; //Method clojure/core$first.invokeStatic:(Ljava/lang/Object;)Ljava/lang/Object;
30: areturn
</pre>

Line number 22 is an unreachable goto given the prior goto on line 19.





[CLJ-274] cannot close over mutable fields (in deftype) Created: 23/Feb/10  Updated: 03/Sep/13

Status: Open
Project: Clojure
Component/s: None
Affects Version/s: None
Fix Version/s: Backlog

Type: Defect Priority: Major
Reporter: Anonymous Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: deftype

Approval: Vetted

 Description   

Simplest case:

user=>
(deftype Bench [#^{:unsynchronized-mutable true} val]
Runnable
(run [_]
(fn [] (set! val 5))))

java.lang.IllegalArgumentException: Cannot assign to non-mutable: val (NO_SOURCE_FILE:5)

Functions should be able to mutate mutable fields in their surrounding deftype (just like inner classes do in Java).

Filed as bug, because the loop special form expands into a fn form sometimes:

user=>
(deftype Bench [#^{:unsynchronized-mutable true} val]
Runnable
(run [_]
(let [x (loop [] (set! val 5))])))
java.lang.IllegalArgumentException: Cannot assign to non-mutable: val (NO_SOURCE_FILE:9)



 Comments   
Comment by Assembla Importer [ 01/Oct/10 9:35 AM ]

Converted from http://www.assembla.com/spaces/clojure/tickets/274

Comment by Assembla Importer [ 01/Oct/10 9:35 AM ]

donmullen said: Updated each run to [_] for new syntax.

Now gives exception listed.

Comment by Assembla Importer [ 01/Oct/10 9:35 AM ]

richhickey said: We're not going to allow closing over mutable fields. Instead we'll have to generate something other than fn for loops et al used as expressions. Not going to come before cinc





[CLJ-415] smarter assert (prints locals) Created: 29/Jul/10  Updated: 03/Sep/13

Status: Open
Project: Clojure
Component/s: None
Affects Version/s: None
Fix Version/s: Backlog

Type: Enhancement Priority: Major
Reporter: Assembla Importer Assignee: Unassigned
Resolution: Unresolved Votes: 2
Labels: errormsgs

Attachments: Text File clj-415-assert-prints-locals-v1.txt    
Approval: Vetted
Waiting On: Rich Hickey

 Description   

Here is an implementation you can paste into a repl. Feedback wanted:

(defn ^{:private true} local-bindings
  "Produces a map of the names of local bindings to their values."
  [env]
  (let [symbols (map key env)]
    (zipmap (map (fn [sym] `(quote ~sym)) symbols) symbols)))

(defmacro assert
  "Evaluates expr and throws an exception if it does not evaluate to
 logical true."
  {:added "1.0"}
  [x]
  (when *assert*
    (let [bindings (local-bindings &env)]
      `(when-not ~x
         (let [sep# (System/getProperty "line.separator")]
           (throw (AssertionError. (apply str "Assert failed: " (pr-str '~x) sep#
                                          (map (fn [[k# v#]] (str "\t" k# " : " v# sep#)) ~bindings)))))))))


 Comments   
Comment by Assembla Importer [ 24/Aug/10 5:41 PM ]

Converted from http://www.assembla.com/spaces/clojure/tickets/415

Comment by Assembla Importer [ 24/Aug/10 5:41 PM ]

alexdmiller said: A simple example I tried for illustration:

user=> (let [a 1 b 2] (assert (= a b)))
#<CompilerException java.lang.AssertionError: Assert failed: (= a b)
 a : 1
 b : 2
Comment by Assembla Importer [ 24/Aug/10 5:41 PM ]

fogus said: Of course it's weird if you do something like:

(let [x 1 y 2 z 3 a 1 b 2 c 3] (assert (= x y)))
java.lang.AssertionError: Assert failed: (= x y)
 x : 1
 y : 2
 z : 3
 a : 1
 b : 2
 c : 3
 (NO_SOURCE_FILE:0)
</code></pre>

So maybe it could be slightly changed to:
<pre><code>(defmacro assert
  "Evaluates expr and throws an exception if it does not evaluate to logical true."
  {:added "1.0"}
  [x]
  (when *assert*
    (let [bindings (local-bindings &env)]
      `(when-not ~x
         (let [sep#  (System/getProperty "line.separator")
               form# '~x]
           (throw (AssertionError. (apply str "Assert failed: " (pr-str form#) sep#
                                          (map (fn [[k# v#]] 
                                                 (when (some #{k#} form#) 
                                                   (str "\t" k# " : " v# sep#))) 
                                               ~bindings)))))))))
</code></pre>

So that. now it's just:
<pre><code>(let [x 1 y 2 z 3 a 1 b 2 c 3] (assert (= x y)))
java.lang.AssertionError: Assert failed: (= x y)
 x : 1
 y : 2
 (NO_SOURCE_FILE:0)

:f

Comment by Assembla Importer [ 24/Aug/10 5:41 PM ]

fogus said: Hmmm, but that fails entirely for: (let [x 1 y 2 z 3 a 1 b 2 c 3] (assert (= [x y] [a c]))). So maybe it's better just to print all of the locals unless you really want to get complicated.
:f

Comment by Assembla Importer [ 24/Aug/10 5:41 PM ]

jawolfe said: See also some comments in:

http://groups.google.com/group/clojure-dev/browse_frm/thread/68d49cd7eb4a4899/9afc6be4d3f8ae27?lnk=gst&q=assert#9afc6be4d3f8ae27

Plus one more suggestion to add to the mix: in addition to / instead of printing the locals, how about saving them somewhere. For example, the var assert-bindings could be bound to the map of locals. This way you don't run afoul of infinite/very large sequences, and allow the user to do more detailed interrogation of the bad values (especially useful when some of the locals print opaquely).

Comment by Assembla Importer [ 24/Aug/10 5:41 PM ]

stuart.sierra said: Another approach, which I wil willingly donate:
http://github.com/stuartsierra/lazytest/blob/master/src/main/clojure/lazytest/expect.clj

Comment by Jeff Weiss [ 15/Dec/10 1:33 PM ]

There's one more tweak to fogus's last comment, which I'm actually using. You need to flatten the quoted form before you can use 'some' to check whether the local was used in the form:

(defmacro assert
  "Evaluates expr and throws an exception if it does not evaluate to logical true."
  {:added "1.0"}
  [x]
  (when *assert*
    (let [bindings (local-bindings &env)]
      `(when-not ~x
         (let [sep#  (System/getProperty "line.separator")
               form# '~x]
           (throw (AssertionError. (apply str "Assert failed: " (pr-str form#) sep#
                                          (map (fn [[k# v#]] 
                                                 (when (some #{k#} (flatten form#)) 
                                                   (str "\t" k# " : " v# sep#))) 
                                               ~bindings)))))))))
Comment by Stuart Halloway [ 04/Jan/11 8:31 PM ]

I am holding off on this until we have more solidity around http://dev.clojure.org/display/design/Error+Handling. (Considering, for instance, having all exceptions thrown from Clojure provide access to locals.)

When my pipe dream fades I will come back and screen this before the next release.

Comment by Stuart Halloway [ 28/Jan/11 1:14 PM ]

Why try to guess what someone wants to do with the locals (or any other context, for that matter) when you can specify a callback (see below). This would have been useful last week when I had an assertion that failed only on the CI box, where no debugger is available.

Rich, at the risk of beating a dead horse, I still think this is a good idea. Debuggers are not always available, and this is an example of where a Lisp is intrinsically capable of providing better information than can be had in other environments. If you want a patch for the code below please mark waiting on me, otherwise please decline this ticket so I stop looking at it.

(def ^:dynamic *assert-handler* nil)

(defn ^{:private true} local-bindings
  "Produces a map of the names of local bindings to their values."
  [env]
  (let [symbols (map key env)]
    (zipmap (map (fn [sym] `(quote ~sym)) symbols) symbols)))

(defmacro assert
  [x]
  (when *assert*
    (let [bindings (local-bindings &env)]
      `(when-not ~x
         (let [sep#  (System/getProperty "line.separator")
               form# '~x]
           (if *assert-handler*
             (*assert-handler* form# ~bindings)
             (throw (AssertionError. (apply str "Assert failed: " (pr-str form#) sep#
                                            (map (fn [[k# v#]] 
                                                   (when (some #{k#} (flatten form#)) 
                                                     (str "\t" k# " : " v# sep#))) 
                                                 ~bindings))))))))))
Comment by Jeff Weiss [ 27/May/11 8:16 AM ]

A slight improvement I made in my own version of this code: flatten does not affect set literals. So if you do (assert (some #{x} [a b c d])) the value of x will not be printed. Here's a modified flatten that does the job:

(defn symbols [sexp]
  "Returns just the symbols from the expression, including those
   inside literals (sets, maps, lists, vectors)."
  (distinct (filter symbol? (tree-seq coll? seq sexp))))
Comment by Andy Fingerhut [ 18/Nov/12 1:06 AM ]

Attaching git format patch clj-415-assert-prints-locals-v1.txt of Stuart Halloway's version of this idea. I'm not advocating it over the other variations, just getting a file attached to the JIRA ticket.





[CLJ-445] Method/Constructor resolution does not factor in widening conversion of primitive args Created: 29/Sep/10  Updated: 27/Jul/13

Status: In Progress
Project: Clojure
Component/s: None
Affects Version/s: None
Fix Version/s: Backlog

Type: Enhancement Priority: Major
Reporter: Alexander Taggart Assignee: Alexander Taggart
Resolution: Unresolved Votes: 3
Labels: None

Attachments: Text File clj-445-prim-conversion-update-2-patch.txt     Text File prim-conversion.patch     Text File prim-conversion-update-1.patch     Text File reorg-reflector.patch    
Approval: Vetted

 Description   

Problem:
When making java calls (or inlined functions), if both args and param are primitive, no widening conversion is used to locate the proper overloaded method/constructor.

Examples:

user=> (Integer. (byte 0))
java.lang.IllegalArgumentException: No matching ctor found for class java.lang.Integer (NO_SOURCE_FILE:0)
</code></pre>
The above occurs because there is no Integer(byte) constructor, though it should match on Integer(int).
<pre><code>user=> (bit-shift-left (byte 1) 1)
Reflection warning, NO_SOURCE_PATH:3 - call to shiftLeft can't be resolved.
2

In the above, a call is made via reflection to Numbers.shiftLeft(Object, Object) and its associated auto-boxing, instead of directly to the perfectly adequate Numbers.shiftLeft(long, int).

Workarounds:
Explicitly casting to the formal type.

Ancillary benefits of fixing:
It would also reduce the amount of method overloading, e.g., RT.intCast(char), intCast(byte), intCast(short), could all be removed, since such calls would pass to RT.intCast(int).



 Comments   
Comment by Assembla Importer [ 23/Oct/10 6:43 PM ]

Converted from http://www.assembla.com/spaces/clojure/tickets/445
Attachments:
fixbug445.diff - https://www.assembla.com/spaces/clojure/documents/b6gDSUZOur36b9eJe5cbCb/download/b6gDSUZOur36b9eJe5cbCb

Comment by Assembla Importer [ 23/Oct/10 6:43 PM ]

ataggart said: [file:b6gDSUZOur36b9eJe5cbCb]

Comment by Assembla Importer [ 23/Oct/10 6:43 PM ]

ataggart said: Also fixes #446.

Comment by Stuart Halloway [ 03/Dec/10 12:50 PM ]

The patch is causing a test failure

[java] Exception in thread "main" java.lang.IllegalArgumentException: 
     More than one matching method found: equiv, compiling:(clojure/pprint/cl_format.clj:428)

Can you take a look?

Comment by Stuart Halloway [ 28/Jan/11 12:30 PM ]

The failing test happens when trying to find the correct equiv for signature (Number, long). Is the compiler wrong to propose this signature, or is the resolution method wrong in not having an answer? (It thinks two signatures are tied: (Object, long) and (Number, Number).)

Exception in thread "main" java.lang.IllegalArgumentException: More than one matching method found: equiv, compiling:(clojure/pprint/cl_format.clj:428)
	at clojure.lang.Compiler.analyzeSeq(Compiler.java:6062)
	at clojure.lang.Compiler.analyze(Compiler.java:5866)
	at clojure.lang.Compiler.analyze(Compiler.java:5827)
	at clojure.lang.Compiler.analyzeSeq(Compiler.java:6050)
	at clojure.lang.Compiler.analyze(Compiler.java:5866)
	at clojure.lang.Compiler.access$100(Compiler.java:35)
	at clojure.lang.Compiler$LetExpr$Parser.parse(Compiler.java:5492)
	at clojure.lang.Compiler.analyzeSeq(Compiler.java:6055)
	at clojure.lang.Compiler.analyze(Compiler.java:5866)
	at clojure.lang.Compiler.analyzeSeq(Compiler.java:6043)
	at clojure.lang.Compiler.analyze(Compiler.java:5866)
	at clojure.lang.Compiler.analyzeSeq(Compiler.java:6043)
	at clojure.lang.Compiler.analyze(Compiler.java:5866)
	at clojure.lang.Compiler.analyze(Compiler.java:5827)
	at clojure.lang.Compiler$IfExpr$Parser.parse(Compiler.java:2372)
	at clojure.lang.Compiler.analyzeSeq(Compiler.java:6055)
	at clojure.lang.Compiler.analyze(Compiler.java:5866)
	at clojure.lang.Compiler.analyzeSeq(Compiler.java:6043)
	at clojure.lang.Compiler.analyze(Compiler.java:5866)
	at clojure.lang.Compiler.analyze(Compiler.java:5827)
	at clojure.lang.Compiler$InvokeExpr.parse(Compiler.java:3277)
	at clojure.lang.Compiler.analyzeSeq(Compiler.java:6057)
	at clojure.lang.Compiler.analyze(Compiler.java:5866)
	at clojure.lang.Compiler.analyze(Compiler.java:5827)
	at clojure.lang.Compiler$BodyExpr$Parser.parse(Compiler.java:5231)
	at clojure.lang.Compiler$LetExpr$Parser.parse(Compiler.java:5527)
	at clojure.lang.Compiler.analyzeSeq(Compiler.java:6055)
	at clojure.lang.Compiler.analyze(Compiler.java:5866)
	at clojure.lang.Compiler.analyzeSeq(Compiler.java:6043)
	at clojure.lang.Compiler.analyze(Compiler.java:5866)
	at clojure.lang.Compiler.analyze(Compiler.java:5827)
	at clojure.lang.Compiler$BodyExpr$Parser.parse(Compiler.java:5231)
	at clojure.lang.Compiler.analyzeSeq(Compiler.java:6055)
	at clojure.lang.Compiler.analyze(Compiler.java:5866)
	at clojure.lang.Compiler.analyze(Compiler.java:5827)
	at clojure.lang.Compiler$IfExpr$Parser.parse(Compiler.java:2385)
	at clojure.lang.Compiler.analyzeSeq(Compiler.java:6055)
	at clojure.lang.Compiler.analyze(Compiler.java:5866)
	at clojure.lang.Compiler.analyze(Compiler.java:5827)
	at clojure.lang.Compiler$BodyExpr$Parser.parse(Compiler.java:5231)
	at clojure.lang.Compiler$LetExpr$Parser.parse(Compiler.java:5527)
	at clojure.lang.Compiler.analyzeSeq(Compiler.java:6055)
	at clojure.lang.Compiler.analyze(Compiler.java:5866)
	at clojure.lang.Compiler.analyzeSeq(Compiler.java:6043)
	at clojure.lang.Compiler.analyze(Compiler.java:5866)
	at clojure.lang.Compiler.analyze(Compiler.java:5827)
	at clojure.lang.Compiler$IfExpr$Parser.parse(Compiler.java:2385)
	at clojure.lang.Compiler.analyzeSeq(Compiler.java:6055)
	at clojure.lang.Compiler.analyze(Compiler.java:5866)
	at clojure.lang.Compiler.analyze(Compiler.java:5827)
	at clojure.lang.Compiler$BodyExpr$Parser.parse(Compiler.java:5231)
	at clojure.lang.Compiler$LetExpr$Parser.parse(Compiler.java:5527)
	at clojure.lang.Compiler.analyzeSeq(Compiler.java:6055)
	at clojure.lang.Compiler.analyze(Compiler.java:5866)
	at clojure.lang.Compiler.analyzeSeq(Compiler.java:6043)
	at clojure.lang.Compiler.analyze(Compiler.java:5866)
	at clojure.lang.Compiler.analyze(Compiler.java:5827)
	at clojure.lang.Compiler$BodyExpr$Parser.parse(Compiler.java:5231)
	at clojure.lang.Compiler$FnMethod.parse(Compiler.java:4667)
	at clojure.lang.Compiler$FnExpr.parse(Compiler.java:3397)
	at clojure.lang.Compiler.analyzeSeq(Compiler.java:6053)
	at clojure.lang.Compiler.analyze(Compiler.java:5866)
	at clojure.lang.Compiler.analyzeSeq(Compiler.java:6043)
	at clojure.lang.Compiler.analyze(Compiler.java:5866)
	at clojure.lang.Compiler.access$100(Compiler.java:35)
	at clojure.lang.Compiler$DefExpr$Parser.parse(Compiler.java:480)
	at clojure.lang.Compiler.analyzeSeq(Compiler.java:6055)
	at clojure.lang.Compiler.analyze(Compiler.java:5866)
	at clojure.lang.Compiler.analyze(Compiler.java:5827)
	at clojure.lang.Compiler.eval(Compiler.java:6114)
	at clojure.lang.Compiler.load(Compiler.java:6545)
	at clojure.lang.RT.loadResourceScript(RT.java:340)
	at clojure.lang.RT.loadResourceScript(RT.java:331)
	at clojure.lang.RT.load(RT.java:409)
	at clojure.lang.RT.load(RT.java:381)
	at clojure.core$load$fn__1427.invoke(core.clj:5308)
	at clojure.core$load.doInvoke(core.clj:5307)
	at clojure.lang.RestFn.invoke(RestFn.java:409)
	at clojure.pprint$eval3969.invoke(pprint.clj:46)
	at clojure.lang.Compiler.eval(Compiler.java:6110)
	at clojure.lang.Compiler.load(Compiler.java:6545)
	at clojure.lang.RT.loadResourceScript(RT.java:340)
	at clojure.lang.RT.loadResourceScript(RT.java:331)
	at clojure.lang.RT.load(RT.java:409)
	at clojure.lang.RT.load(RT.java:381)
	at clojure.core$load$fn__1427.invoke(core.clj:5308)
	at clojure.core$load.doInvoke(core.clj:5307)
	at clojure.lang.RestFn.invoke(RestFn.java:409)
	at clojure.core$load_one.invoke(core.clj:5132)
	at clojure.core$load_lib.doInvoke(core.clj:5169)
	at clojure.lang.RestFn.applyTo(RestFn.java:143)
	at sun.reflect.GeneratedMethodAccessor11.invoke(Unknown Source)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25)
	at java.lang.reflect.Method.invoke(Method.java:597)
	at clojure.lang.Reflector.invokeMatchingMethod(Reflector.java:77)
	at clojure.lang.Reflector.invokeInstanceMethod(Reflector.java:28)
	at clojure.core$apply.invoke(core.clj:602)
	at clojure.core$load_libs.doInvoke(core.clj:5203)
	at clojure.lang.RestFn.applyTo(RestFn.java:138)
	at sun.reflect.GeneratedMethodAccessor11.invoke(Unknown Source)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25)
	at java.lang.reflect.Method.invoke(Method.java:597)
	at clojure.lang.Reflector.invokeMatchingMethod(Reflector.java:77)
	at clojure.lang.Reflector.invokeInstanceMethod(Reflector.java:28)
	at clojure.core$apply.invoke(core.clj:604)
	at clojure.core$use.doInvoke(core.clj:5283)
	at clojure.lang.RestFn.invoke(RestFn.java:409)
	at clojure.main$repl.doInvoke(main.clj:196)
	at clojure.lang.RestFn.invoke(RestFn.java:422)
	at clojure.main$repl_opt.invoke(main.clj:267)
	at clojure.main$main.doInvoke(main.clj:362)
	at clojure.lang.RestFn.invoke(RestFn.java:409)
	at clojure.lang.Var.invoke(Var.java:401)
	at clojure.lang.AFn.applyToHelper(AFn.java:163)
	at clojure.lang.Var.applyTo(Var.java:518)
	at clojure.main.main(main.java:37)
Caused by: java.lang.IllegalArgumentException: More than one matching method found: equiv
	at clojure.lang.Reflector.getMatchingParams(Reflector.java:639)
	at clojure.lang.Reflector.getMatchingParams(Reflector.java:578)
	at clojure.lang.Reflector.getMatchingMethod(Reflector.java:569)
	at clojure.lang.Compiler$StaticMethodExpr.<init>(Compiler.java:1439)
	at clojure.lang.Compiler$HostExpr$Parser.parse(Compiler.java:896)
	at clojure.lang.Compiler.analyzeSeq(Compiler.java:6055)
	... 115 more
Comment by Alexander Taggart [ 08/Feb/11 6:27 PM ]

In working on implementing support for vararg methods, I found a number of flaws with the previous solutions. Please disregard them.

I've attached a single patch (reflector-compiler-numbers.diff) which is a rather substantial overhaul of the Reflector code, with some enhancements to the Compiler and Numbers code.

The patch notes:

  • Moved reflection functionality from Compiler to Reflector.
  • Reflector supports finding overloaded methods by widening conversion, boxing conversion, and casting.
  • During compilation Reflector attempts to find best wildcard match.
  • Reflector refers to *unchecked-math* when reflectively invoking methods and constructors.
  • Both Reflector and Compiler support variable arity java methods and constructor; backwards compatible with passing an array or nil in the vararg slot.
  • Added more informative error messages to Reflector.
  • Added tests to clojure.test-clojure.reflector.
  • Altered overloaded functions of clojure.lang.Numbers to service Object/double/long params; fixes some ambiguity issues and avoids unnecessary boxing in some cases.
  • Patch closes issues 380, 440, 445, 666, and possibly 259 (not enough detail provided).
Comment by Alexander Taggart [ 10/Feb/11 7:35 PM ]

Updated patch to fix a bug where a concrete class with multiple identical Methods (e.g., one from an interface, another from an abstract class) would result in ambiguity. Now resolved by arbitrary selection (this is what the original code did as well albeit not explicitly).

Comment by Alexander Taggart [ 25/Feb/11 9:29 PM ]

Updated patch to work with latest master branch.

Comment by Stuart Halloway [ 06/Mar/11 1:54 PM ]

patch appears to be missing test file clojure/test_clojure/reflector.clj.

Comment by Alexander Taggart [ 06/Mar/11 2:39 PM ]

Bit by git.

Patch corrected to contain clojure.test-clojure.reflector.

Comment by Stuart Halloway [ 11/Mar/11 10:30 AM ]

Rich: I verified that the patch applied but reviewed only briefly, knowing you will want to go over this one closely.

Comment by Stuart Halloway [ 11/Mar/11 10:55 AM ]

After applying this patch, I am getting method missing errors:

java.lang.NoSuchMethodError: clojure.lang.Numbers.lt(JLjava/lang/Object;)

but only when using compiled code, e.g. the same code works in the REPL and then fails after compilation. Haven't been able to isolate an example that I can share here yet, but hoping this will cause someone to have an "a, hah" moment...

Comment by Alexander Taggart [ 02/Apr/11 12:55 PM ]

The patch now contains only the minimal changes needed to support widening conversion. Cleanup of Numbers overloads, etc., can wait until this patch gets applied. The vararg support is in a separate patch on CLJ-440.

Comment by Christopher Redinger [ 15/Apr/11 12:50 PM ]

Please test patch

Comment by Christopher Redinger [ 21/Apr/11 11:02 AM ]

FYI: Patch applies cleanly on master and all tests pass as of 4/21 (2011)

Comment by Christopher Redinger [ 22/Apr/11 9:57 AM ]

This work is too big to take into the 1.3 beta right now. We'll revisit for a future release.

Comment by Alexander Taggart [ 28/Apr/11 1:19 PM ]

To better facilitate understanding of the changes, I've broken them up into two patches, each with a number of isolable, incremental commits:

reorg-reflector.patch: Moves the reflection/invocation code from Compiler to Reflector, and eliminates redundant code. The result is a single code base for resolving methods/constructors, which will allow for altering that mechanism without excess external coordination. This contains no behaviour changes.

prim-conversion.patch: Depends on the above. Alters the method/constructor resolution process:

  • more consistent with java resolution, especially when calling pre-1.5 APIs
  • adds support for widening conversion of primitive numerics of the same category (this is more strict than java, and more clojuresque)
  • adds support for wildcard matches at compile-time (i.e., you don't need to type-hint every arg to avoid reflection).

This also provides a base to add further features, e.g., CLJ-666.

Comment by Alexander Taggart [ 29/Apr/11 3:01 PM ]

It's documented in situ, but here are the conversion rules that the reflector uses to find methods:

  1. By Type:
    • object to ancestor type
    • primitive to a wider primitive of the same numeric category (new)
  2. Boxing:
    • boxed number to its primitive
    • boxed number to a wider primitive of the same numeric category (new for Short and Byte args)
    • primitive to its boxed value
    • primitive to Number or Object (new)
  3. Casting:
    • long to int
    • double to float
Comment by Alexander Taggart [ 10/May/11 3:13 PM ]

prim-conversion-update-1.patch applies to current master.

Comment by Alexander Taggart [ 11/May/11 2:15 PM ]

Created CLJ-792 for the reflector reorg.

Comment by Stuart Sierra [ 17/Feb/12 2:29 PM ]

prim-conversion-update-1.patch does not apply as of f5bcf64.

Is CLJ-792 now a prerequisite of this ticket?

Comment by Alexander Taggart [ 17/Feb/12 3:15 PM ]

Yes, after the original patch was deemed "too big".

After this much time with no action from TPTB, feel free to kill both tickets.

Comment by Andy Fingerhut [ 20/Feb/12 2:04 PM ]

Again, not sure if this is any help, but I've tested starting from Clojure head as of Feb 20, 2012, applying clj-792-reorg-reflector-patch2.txt attached to CLJ-792, and then applying clj-445-prim-conversion-update-2-patch.txt attached to this ticket, and the result compiles and passes all but 2 tests. I don't know whether those failures are easy to fix or not, or whether issues may have been introduced with these patches.





[CLJ-701] Compiler loses 'loop's return type in some cases Created: 03/Jan/11  Updated: 21/Jul/14

Status: Open
Project: Clojure
Component/s: None
Affects Version/s: Backlog
Fix Version/s: Release 1.7

Type: Defect Priority: Major
Reporter: Chouser Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None
Environment:

Clojure commit 9052ca1854b7b6202dba21fe2a45183a4534c501, version 1.3.0-master-SNAPSHOT


Approval: Vetted

 Description   
(set! *warn-on-reflection* true)
(fn [] (loop [b 0] (recur (loop [a 1] a))))

Generates the following warnings:

recur arg for primitive local: b is not matching primitive, had: Object, needed: long
Auto-boxing loop arg: b

This is interesting for several reasons. For one, if the arg to recur is a let form, there is no warning:

(fn [] (loop [b 0] (recur (let [a 1] a))))

Also, the compiler appears to understand the return type of loop forms just fine:

(use '[clojure.contrib.repl-utils :only [expression-info]])
(expression-info '(loop [a 1] a))
;=> {:class long, :primitive? true}

The problem can of course be worked around using an explicit cast on the loop form:

(fn [] (loop [b 0] (recur (long (loop [a 1] a)))))

Reported by leafw in IRC: http://clojure-log.n01se.net/date/2011-01-03.html#10:31



 Comments   
Comment by a_strange_guy [ 03/Jan/11 4:36 PM ]

The problem is that a 'loop form gets converted into an anonymous fn that gets called immediately, when the loop is in a expression context (eg. its return value is needed, but not as the return value of a method/fn).

so

(fn [] (loop [b 0] (recur (loop [a 1] a))))

gets converted into

(fn [] (loop [b 0] (recur ((fn [] (loop [a 1] a))))))

see the code in the compiler:
http://github.com/clojure/clojure/blob/master/src/jvm/clojure/lang/Compiler.java#L5572

this conversion already bites you if you have mutable fields in a deftype and want to 'set! them in a loop

http://dev.clojure.org/jira/browse/CLJ-274

Comment by Christophe Grand [ 23/Nov/12 2:28 AM ]

loops in expression context are lifted into fns because else Hotspot doesn't optimize them.
This causes several problems:

  • type inference doesn't propagate outside of the loop[1]
  • the return value is never a primitive
  • mutable fields are inaccessible
  • surprise allocation of one closure objects each time the loop is entered.

Adressing all those problems isn't easy.
One can compute the type of the loop and emit a type hint but it works only with reference types. To make it works with primitive, primitie fns aren't enough since they return only long/double: you have to add explicit casts.
So solving the first two points can be done in a rather lccal way.
The two other points require more impacting changes, the goal would be to emit a method rather than a fn. So it means at the very least changing ObjExpr and adding a new subclassof ObjMethod.

[1] beware of CLJ-1111 when testing.

Comment by Alex Miller [ 21/Oct/13 10:28 PM ]

I don't think this is going to make it into 1.6, so removing the 1.6 tag.

Comment by Kevin Downey [ 21/Jul/14 7:14 PM ]

an immediate solution to this might be to hoist loops out in to distinct non-ifn types generated by the compiler with an invoke method that is typed to return the getJavaClass() type of the expression, that would give us the simplifying benefits of hoisting the code out and free use from the Object semantics of ifn





[CLJ-1192] vec function is substantially slower than into function Created: 06/Apr/13  Updated: 18/May/14

Status: Open
Project: Clojure
Component/s: None
Affects Version/s: Release 1.5
Fix Version/s: Release 1.7

Type: Enhancement Priority: Major
Reporter: Luke VanderHart Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: performance

Approval: Vetted

 Description   

(vec coll) and (into [] coll) do exactly the same thing. However, due to into using transients, it is substantially faster. On my machine:

(time (dotimes [_ 100] (vec (range 100000))))
"Elapsed time: 732.56 msecs"

(time (dotimes [_ 100] (into [] (range 100000))))
"Elapsed time: 491.411 msecs"

This is consistently repeatable.

Since vec's sole purpose is to transform collections into vectors, it should do so at the maximum speed available.



 Comments   
Comment by Andy Fingerhut [ 07/Apr/13 5:50 PM ]

I am pretty sure that Clojure 1.5.1 also uses transient vectors for (vec (range n)) (probably also some earlier versions of Clojure, too).

Look at vec in core.clj. It checks whether its arg is a java.util.Collection, which lazy seqs are, so calls (clojure.lang.LazilyPersistentVector/create coll).

LazilyPersistentVector's create method checks whether its argument is an ISeq, which lazy seqs are, so it calls PersistentVector.create(RT.seq(coll)).

All 3 of PersistentVector's create() methods use transient vectors to build up the result.

I suspect the difference in run times are not because of transients or not, but because of the way into uses reduce, and perhaps may also have something to do with the perhaps-unnecessary call to RT.seq in LazilyPersistentVector's create method (in this case, at least – it is likely needed for other types of arguments).

Comment by Alan Malloy [ 14/Jun/13 2:17 PM ]

I'm pretty sure the difference is that into uses reduce: since reducers were added in 1.5, chunked sequences know how to reduce themselves without creating unnecessary cons cells. PersistentVector/create doesn't use reduce, so it has to allocate a cons cell for each item in the sequence.

Comment by Gary Fredericks [ 08/Sep/13 1:55 PM ]

Is there any downside to (defn vec [coll] (into [] coll)) (or the inlined equivalent)?

Comment by Ghadi Shayban [ 11/Apr/14 5:13 PM ]

While I agree that there are improvements and possibly low-hanging fruit, FWIW https://github.com/clojure/tools.analyzer/commit/cf7dda81a22f4c9c1fe64c699ca17e7deed61db4#commitcomment-5989545

showed a 5% slowdown from a few callsites in tools.analyzer.

This ticket's benchmark is incomplete in that it covers a single type of argument (chunked range), and flawed as it timing the expense of realizing the range. (That could be a legit benchmark case, but it shouldn't be the only one).

Sorry to rain on a parade. I promise like speed too!

Comment by Greg Chapman [ 25/Apr/14 5:23 PM ]

One thing to note is that vec has a subtle difference from into when the collection is an Object array of length <= 32. In that case, vec aliases the supplied array, rather than copying it (this is noted in the warning here: http://clojuredocs.org/clojure_core/clojure.core/vec). I believe I read some place that this behavior is intentional, but I can't find the citation.

Comment by Andy Fingerhut [ 25/Apr/14 10:18 PM ]

Greg, CLJ-893 might be what you remember. That is the ticket that was closed by a patch updating the documentation of vec.

Comment by Mike Anderson [ 18/May/14 7:41 AM ]

I think there are quite a few performance improvements that can be made to vec in general. For example, if given a List it should use PersistentVector.create(List) rather than producing an unnecessary seq, which appears to be the case at the moment. Also it should probably return the same object if passed an existing IPersistentVector.

Basically there are a number of cases that we could be handling more efficiently....

I'm taking a look at this now.... will propose a quick patch if it seems there is a good solution.





[CLJ-1208] Namespace is not loaded on defrecord class init Created: 03/May/13  Updated: 10/Jun/14

Status: Open
Project: Clojure
Component/s: None
Affects Version/s: None
Fix Version/s: Release 1.7

Type: Enhancement Priority: Major
Reporter: Tim McCormack Assignee: Unassigned
Resolution: Unresolved Votes: 4
Labels: defrecord

Approval: Vetted

 Description   

As a user of Clojure interop from Java, I want defrecords (and deftypes?) to load their namespaces upon class initialization so that I can simply construct and use AOT'd record classes without manually requiring their namespaces first.

Calling the defrecord's constructor may or may not result in "Attempting to call unbound fn" exceptions, depending on what code has already been run.

This issue has been raised several times over the years, but I could not find an existing ticket for it:






[CLJ-69] GC Issue 66: Add "keyset" to Clojure; make .keySet for APersistentMap return IPersistentSet Created: 17/Jun/09  Updated: 27/Jul/13

Status: In Progress
Project: Clojure
Component/s: None
Affects Version/s: Backlog
Fix Version/s: Backlog

Type: Enhancement Priority: Minor
Reporter: Assembla Importer Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None

Approval: Vetted

 Description   
Reported by wolfe.a.jason, Feb 04, 2009

Describe the feature/change.

Add "keyset" to Clojure; make .keySet for APersistentMap return an
IPersistentSet

Was this discussed on the group? If so, please provide a link to the
discussion:

http://groups.google.com/group/clojure/browse_thread/thread/66e708e477ae992f/ff3d8d588068b60e?hl=en#ff3d8d588068b60e

-----------------------------------------------------

A patch is attached.  Some notes:

I chose to add a "keyset" function, rather than change the existing "keys",
so as to avoid breaking anything.

The corresponding RT.keyset function just calls .keySet on the argument.
I would have liked to have "keyset" return an IPersistentSet even when
passed a (non-Clojure) java.util.Map, but this seems impossible to do in
sublinear time because of essentially the same limitation mentioned in the
above thread (the Map interface does not support getKey() or entryAt()) --
assuming, again, that "get" is supposed to return the actual (identical?)
key in a set, and not just an .equal key.

I then changed the implementation of .keySet for APersistentMap to
essentially copy APersistentSet.  A more concise alternative would have
been to extend APersistentSet and override the .get method, but that made
me a bit nervous (since if APeristentSet changed this could break). 

Anyway, this is my first patch for the Java side of Clojure, and I'm not
yet solid on the conventions and aesthetics, so
comments/questions/criticisms/requests for revisions are very welcome.


 Comments   
Comment by Assembla Importer [ 28/Sep/10 7:00 AM ]

Converted from http://www.assembla.com/spaces/clojure/tickets/69
Attachments:
keyset.patch - https://www.assembla.com/spaces/clojure/documents/dKgE6mw3Gr3O2PeJe5afGb/download/dKgE6mw3Gr3O2PeJe5afGb

Comment by Assembla Importer [ 28/Sep/10 7:00 AM ]

oranenj said: [file:dKgE6mw3Gr3O2PeJe5afGb]

Comment by Assembla Importer [ 28/Sep/10 7:00 AM ]

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)

Comment by Stuart Halloway [ 03/Dec/10 1:12 PM ]

patch not in correct format





[CLJ-84] GC Issue 81: compile gen-class fail when class returns self Created: 17/Jun/09  Updated: 27/Jul/13

Status: In Progress
Project: Clojure
Component/s: None
Affects Version/s: Backlog
Fix Version/s: Backlog

Type: Defect Priority: Minor
Reporter: Assembla Importer Assignee: Rich Hickey
Resolution: Unresolved Votes: 0
Labels: None

Approval: Vetted
Waiting On: Chouser

 Description   
Reported by davidhaub, Feb 14, 2009

When attempting to compile the following program, clojure fails with a
ClassNotFoundException.  It occurs because one of the methods returns the
same class that is being generated.  If the returnMe method below is
changed to return an Object, the compile succeeds.

Beware when testing! If the classpath contains a class file (say from a
prior successful build when the returnMe method was changed to return an
object), the compile will succeed.  Always clear out the
clojure.compile.path prior to compiling.

;badgenclass.clj
(ns badgenclass
  (:gen-class
     :state state
     :methods
     [[returnMe [] badgenclass]]
     :init init))
(defn -init []
  [[] nil])

(defn -returnMe [this]
  this)

#!/bin/sh
rm -rf classes
mkdir classes
java -cp lib/clojure.jar:classes:. -Dclojure.compile.path=classes \
clojure.lang.Compile badgenclass


Comment 1 by chouser, Mar 07, 2009

Attached is a patch that accepts strings or symbols for parameter and return class
names, and generates the appropriate bytecode without calling Class/forName.  It
fixes this issue, but because 'ns' doesn't resolve :gen-class's arguments, class
names aren't checked as early anymore.  :gen-class-created classes with invalid
parameter or return types can even be instantiated, and no error will be reported
until the broken method is called.

One possible alternative would be to call Class/forName on any symbols given, but
allow strings to use the method given by this patch.  To return your own type, you'd
need a method defined like:

  [returnMe [] "badgenclass"]

Any thoughts?


 Comments   
Comment by Assembla Importer [ 28/Sep/10 5:09 PM ]

Converted from http://www.assembla.com/spaces/clojure/tickets/84
Attachments:
genclass-allow-unresolved-classname.patch - https://www.assembla.com/spaces/clojure/documents/cWS6Aww30r3RbzeJe5afGb/download/cWS6Aww30r3RbzeJe5afGb

Comment by Assembla Importer [ 28/Sep/10 5:09 PM ]

oranenj said: [file:cWS6Aww30r3RbzeJe5afGb]: on comment 1

Comment by Assembla Importer [ 28/Sep/10 5:09 PM ]

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)

Comment by Stuart Halloway [ 30/Oct/10 11:58 AM ]

The approach take in the initial patch (delaying resolution of symbols into classes) is fine: gen-class makes no promise about when this happens, and the dynamic approach feels more consistent with Clojure. I think the proposed (but not implemented) use of string/symbol to control when class names are resolved is a bad idea: magical and not implied by the types.

Needed:

  • update the patch to apply cleanly
  • consider whether totype could live in clojure.reflect.java. (Beware load order dependencies.)
Comment by Chouser [ 30/Oct/10 9:29 PM ]

Wow, 18-month-old patch, back to haunt me for Halloway'een

So what does it mean that the assignee is Rich, but it's waiting on me?

Comment by Stuart Halloway [ 01/Nov/10 9:17 AM ]

I am using Approval = Incomplete plus Waiting On = Someone to let submitters know that there is feedback waiting, and that they can move the ticket forward by acting on it. The distinction is opportunity to contribute (something has been provided to let you move forward) vs. expectation of contribution.





[CLJ-124] GC Issue 120: Determine mechanism for controlling automatic shutdown of Agents, with a default policy and mechanism for changing that policy as needed Created: 17/Jun/09  Updated: 10/Apr/14

Status: Open
Project: Clojure
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Chas Emerick Assignee: Alex Miller
Resolution: Unresolved Votes: 3
Labels: agents

Approval: Vetted
Waiting On: Rich Hickey

 Description   
Reported by cemer...@snowtide.com, Jun 01, 2009

There has been intermittent chatter over the past months from a couple of
people on the group (e.g.
http://groups.google.com/group/clojure/browse_thread/thread/409054e3542adc1f)
and in #clojure about some clojure scripts hanging, either for a constant
time (usually reported as a minute or so with no CPU util) or seemingly
forever (or until someone kills the process).

I just hit a similar situation in our compilation process, which invokes
clojure.lang.Compile from ant.  The build process for this particular
project had taken 15 second or so, but after adding a couple of pmap calls,
that build time jumped to ~1:15, with roughly zero CPU utilization over the
course of that last minute.

Adding a call to Agent.shutdown() in the finally block in
clojure.lang.Compile/main resolved the problem; a patch including this
change is attached.  I wouldn't suspect anyone would have any issues with
such a change.

-----
In general, it doesn't seem like everyone should keep tripping over this
problem in different directions.  It's a very difficult thing to debug if
you're not attuned to how clojure's concurrency primitives work under the
hood, and I would bet that newer users would be particularly affected.

After discussion in #clojure, rhickey suggested adding a
*auto-shutdown-agents* var, which:

- if true when exiting one of the main entry points (clojure.main, or the
legacy script/repl entry points), Agent.shutdown() would be called,
allowing for the clean exit of the application

- would be bound by default to true

- could be easily set to false for anyone with an advanced use-case that
requires agents to remain active after the main thread of the application
exits.

This would obviously not help anyone initializing clojure from a different
entry point, but this may represent the best compromise between
least-surprise and maximal functionality for advanced users.

------

In addition to the above, it perhaps might be worthwhile to change the
keepalive values used to create the Threadpools used by c.l.Actor's
Executors.  Currently, Actor uses a default thread pool executor, which
results in a 60s keepalive.  Lowering this to something much smaller (1s?
5s?) would additionally minimize the impact of Agent's threadpools on Java
applications that embed clojure directly (and would therefore not benefit
from *auto-shutdown-agents* as currently conceived, leading to puzzling
'hanging' behaviour).  I'm not in a position to determine what impact this
would have on performance due to thread churn, but it would at least
minimize what would be perceived as undesirable behaviour by users that are
less familiar with the implementation details of Agent and code that
depends on it.

Comment 1  by cemer...@snowtide.com, Jun 01, 2009

Just FYI, I'd be happy to provide patches for either of the suggestions mentioned
above...


 Comments   
Comment by Assembla Importer [ 24/Aug/10 12:45 AM ]

Converted from http://www.assembla.com/spaces/clojure/tickets/124
Attachments:
compile-agent-shutdown.patch - https://www.assembla.com/spaces/clojure/documents/a56S2ow4ur3O2PeJe5afGb/download/a56S2ow4ur3O2PeJe5afGb
124-compilation.diff - https://www.assembla.com/spaces/clojure/documents/aqn0IGxZSr3RUGeJe5aVNr/download/aqn0IGxZSr3RUGeJe5aVNr

Comment by Assembla Importer [ 24/Aug/10 12:45 AM ]

oranenj said: [file:a56S2ow4ur3O2PeJe5afGb]

Comment by Assembla Importer [ 24/Aug/10 12:45 AM ]

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)

Comment by Assembla Importer [ 24/Aug/10 12:45 AM ]

cemerick said: (In [[r:fa3d24973fc415b35ae6ec8d84b61ace76bd4133]]) Add a call to Agent.shutdown() at the end of clojure.lang.Compile/main Refs #124

Signed-off-by: Chouser <chouser@n01se.net>

Branch: master

Comment by Assembla Importer [ 24/Aug/10 12:45 AM ]

chouser@n01se.net said: I'm closing this ticket to because the attached patch solves a specific problem. I agree that the idea of an auto-shutdown-agents var sounds like a positive compromise. If Rich wants a ticket to track that issue, I think it'd be best to open a new ticket (and perhaps mention this one there) rather than use this ticket to track further changes.

Comment by Assembla Importer [ 24/Aug/10 12:45 AM ]

scgilardi said: With both Java 5 and Java 6 on Mac OS X 10.5 Leopard I'm getting an error when compiling with this change present.

Java 1.5.0_19
Java 1.6.0_13

For example, when building clojure using "ant" from within my clone of the clojure repo:

[java] java.security.AccessControlException: access denied (java.lang.RuntimePermission modifyThread)
[java] at java.security.AccessControlContext.checkPermission(AccessControlContext.java:264)
[java] at java.security.AccessController.checkPermission(AccessController.java:427)
[java] at java.util.concurrent.ThreadPoolExecutor.shutdown(ThreadPoolExecutor.java:894)
[java] at clojure.lang.Agent.shutdown(Agent.java:34)
[java] at clojure.lang.Compile.main(Compile.java:71)

I reproduced this on two Mac OS X 10.5 machines. I'm not aware of having any enhanced security policies along these lines on my machines. The compile goes fine for me with Java 1.6.0_0 on an Ubuntu box.

Comment by Assembla Importer [ 24/Aug/10 12:45 AM ]

chouser@n01se.net said: I had only tested it on my ubuntu box – looks like that was openjdk 1.6.0_0. I'll test again with sun-java5 and sun-java6.

Comment by Assembla Importer [ 24/Aug/10 12:45 AM ]

chouser@n01se.net said: 1.6.0_13 worked fine for me on ubuntu, but 1.5.0_18 generated an the exception Steve pasted. Any suggestions? Should this patch be backed out until someone has a fix?

Comment by Assembla Importer [ 24/Aug/10 12:45 AM ]

achimpassen said: [file:aqn0IGxZSr3RUGeJe5aVNr]

Comment by Assembla Importer [ 24/Aug/10 12:45 AM ]

chouser@n01se.net said: With Achim's patch, clojure compiles for me on ubuntu using java 1.5.0_18 from sun, and still works on 1.6.0_13 sun and 1.6.0_0 openjdk. I don't know anything about ant or the security error, but this is looking good to me.

Comment by Assembla Importer [ 24/Aug/10 12:45 AM ]

achimpassen said: It works for me on 1.6.0_13 and 1.5.0_19 (32 and 64 bit) on OS X 10.5.7.

Comment by Assembla Importer [ 24/Aug/10 12:45 AM ]

chouser@n01se.net said: (In [[r:895b39dabc17b3fd766fdbac3b0757edb0d4b60d]]) Rev fa3d2497 causes compile to fail on some VMs – back it out. Refs #124

Branch: master

Comment by Assembla Importer [ 24/Aug/10 12:45 AM ]

mikehinchey said: I got the same compile error on both 1.5.0_11 and 1.6.0_14 on Windows. Achim's patch fixes both.

See the note for "permissions" on http://ant.apache.org/manual/CoreTasks/java.html . I assume ThreadPoolExecutor.shutdown is the problem, it would shutdown the main Ant thread, so Ant disallows that. Forking avoids the permissions limitation.

In addition, since the build error still resulted in "BUILD SUCCESSFUL", I think failonerror="true" should also be added to the java call so the build would totally fail for such an error.

Comment by Assembla Importer [ 24/Aug/10 12:45 AM ]

chouser@n01se.net said: I don't know if the <java fork=true> patch is a good idea or not, or if there's a better way to solve the original problem.

Chas, I'm kicking back to you, but I guess if you don't want it you can reassign to "nobody".

Comment by Assembla Importer [ 24/Aug/10 12:45 AM ]

richhickey said: Updating tickets (#8, #42, #113, #2, #20, #94, #96, #104, #119, #124, #127, #149, #162)

Comment by Assembla Importer [ 24/Aug/10 12:45 AM ]

shoover said: I'd like to suggest an alternate approach. There are already well-defined and intuitive ways to block on agents and futures. Why not deprecate shutdown-agents and force users to call await and deref if they really want to block? In the pmap situation one would have to evaluate the pmap form.

The System.exit problem goes away if you configure the threadpools to use daemon threads (call new ThreadPoolExecutor and pass a thread factory that creates threads and sets daemon to true). That way the user has an explicit means of blocking and System.exit won't hang.

Comment by Assembla Importer [ 24/Aug/10 12:45 AM ]

alexdmiller said: I blogged about these issues at:
http://tech.puredanger.com/2010/06/08/clojure-agent-thread-pools/

I think that:

  • agent thread pool threads should be named (see ticket #378)
  • agent thread pools must be daemon threads by default
  • having ways to specify an customized executor pool for an agent send/send-off is essential to customize threading behavior
  • (shutdown-agents) should be either deprecated or made less dangerous
Comment by Alexander Taggart [ 11/Jul/11 9:33 PM ]

Rich, what is the intention behind using non-daemon threads in the agent pools?

If it is because daemon threads could terminate before their work is complete, would it be acceptable to add a shutdown hook to ensure against such premature termination? Such a shutdown hook could call Agent.shutdown(), then awaitTermination() on the pools.

Comment by Christopher Redinger [ 27/Nov/12 3:47 PM ]

Moving this ticket out of approval "OK" status, and dropping the priority. These were Assembla import defaults.

Also, Chas gets to be the Reporter now.

Comment by Chas Emerick [ 27/Nov/12 5:56 PM ]

Heh, blast from the past.

The comment import appears to have set their timestamps to the date of the import, so the conversation is pretty hard to follow, and obviously doesn't benefit from the intervening years of experience. In addition, there have been plenty of changes to agents, including some recent enhancements that address some of the pain points that Alex Miller mentioned above.

I propose closing this as 'invalid' or whatever, and opening one or more new issues to track whatever issues still persist (presumably based on fresh ML discussion, etc).

Comment by Andy Fingerhut [ 27/Nov/12 6:11 PM ]

Rereading the original description of this ticket, without reading all of the comments that follow, that description is still right on target for the behavior of latest Clojure master today.

People send messages to the Clojure Google group every couple of months hitting this issue, and one even filed CLJ-959 because of hitting it. I have updated the examples on ClojureDocs.org for future, and also for pmap and clojure.java.shell/sh which use future in their implementations, to warn people about this and explain that they should call (shutdown-agents), but making it unnecessary to call shutdown-agents would be even better, at least as the default behavior. It sounds fine to me to provide a way for experts on thread behavior to change that default behavior if they need to.





[CLJ-211] Support arbitrary functional destructuring via -> and ->> Created: 17/Nov/09  Updated: 27/Jul/13

Status: In Progress
Project: Clojure
Component/s: None
Affects Version/s: None
Fix Version/s: Backlog

Type: Enhancement Priority: Minor
Reporter: Assembla Importer Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None

Approval: Vetted

 Description   

Support arbitrary functional destructuring, that is the use of
any function in any destructuring form to help unpack data in
arbitrary ways.

The discussion began here:
http://clojure-log.n01se.net/date/2009-11-17.html#09:31c

The attached patch implements the spec described here:
http://clojure-log.n01se.net/date/2009-11-17.html#10:50a

That is, the following examples would now work:

user=> (let [(-> str a) 1] a)
"1"

user=> (let [[a (-> str b) c] [1 2]] (list a b c))
(1 "2" nil)

user=> (let [(->> (map int) [a b]) "ab"] (list a b))
(97 98)



 Comments   
Comment by Assembla Importer [ 28/Sep/10 6:57 AM ]

Converted from http://www.assembla.com/spaces/clojure/tickets/211
Attachments:
destructuring-fns.diff - https://www.assembla.com/spaces/clojure/documents/aHWQ_W06Kr3O89eJe5afGb/download/aHWQ_W06Kr3O89eJe5afGb

Comment by Assembla Importer [ 28/Sep/10 6:57 AM ]

chouser@n01se.net said: [file:aHWQ_W06Kr3O89eJe5afGb]: [PATCH] Support -> and ->> in destructuring forms.

Comment by Assembla Importer [ 28/Sep/10 6:57 AM ]

cgrand said: I think the current patch suffers from the problem described here http://groups.google.com/group/clojure-dev/msg/80ba7fad2ff04708 too.

Comment by Assembla Importer [ 28/Sep/10 6:57 AM ]

richhickey said: so, don't use syntax-quote, just use clojure.core/->

Comment by Assembla Importer [ 28/Sep/10 6:57 AM ]

chouser@n01se.net said: Only -> and ->> are actually legal here anyway – if you've locally bound foo to -> there's not really any good reason to think (fn [(foo inc a)] a) should work. And if you've redefined -> or ->> to mean something else in your ns, do we need to catch that at compile time, or is it okay to emit the rearranged code and see what happens?

In short, would '#{> ->> clojure.core/> clojure.core/->>} be sufficient?

Comment by Assembla Importer [ 28/Sep/10 6:57 AM ]

cgrand said: Only -> and ->> are legal here but what if they are aliased or shadowed? Instead of testing the symboil per se I would check, if:

  • the symbol is not in &env
  • the symbol resolve to #'clojure.core/> or #'clojure.core/>>
(when-not (&env (first b)) (#{#'clojure.core/-> #'clojure.core/->>} (resolve (first b))))

but it requires to change destructure's sig to pass the env around

Comment by Stuart Halloway [ 03/Dec/10 1:03 PM ]

Rich: Are you assigned to this by accident? If so, please deassign yourself.





[CLJ-250] debug builds Created: 27/Jan/10  Updated: 23/Oct/13

Status: Open
Project: Clojure
Component/s: None
Affects Version/s: Release 1.2
Fix Version/s: Backlog

Type: Enhancement Priority: Minor
Reporter: Stuart Halloway Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: build

Approval: Vetted
Waiting On: Rich Hickey

 Description   

This ticket includes two patches:

  1. a patch to set assert when clojure.lang.RT loads, based on the presence of system property clojure.debug
  2. expand error messages in assert to include local-bindings</code> (a new macro which wraps the implicit <code>&env)

Things to consider before approving these patches:

  1. should there be an easy Clojure-level way to query if debug is enabled? (checking assert isn't the same, as debug should eventually drive other features)
  2. assertions will now be off by default – this is a change!
  3. is the addition of the name local-bindings to clojure.core cool?


 Comments   
Comment by Assembla Importer [ 24/Aug/10 6:05 AM ]

Converted from http://www.assembla.com/spaces/clojure/tickets/250
Attachments:
add-clojure-debug-flag.patch - https://www.assembla.com/spaces/clojure/documents/aUWn50c64r35E-eJe5aVNr/download/aUWn50c64r35E-eJe5aVNr
assert-report-locals.patch - https://www.assembla.com/spaces/clojure/documents/aUWqLSc64r35E-eJe5aVNr/download/aUWqLSc64r35E-eJe5aVNr

Comment by Stuart Halloway [ 07/Dec/10 8:23 PM ]

Ignore the old patches. Considering the following implementation, please review and then move ticket to waiting on Stu:

  1. RT will check system property "clojure.debug", default to false
  2. property will set the root binding for the current *assert*, plus a new *debug* flag. (Debug builds can and will drive other things than just asserts.)
  3. does Compile.java need to push *assert* or *debug* as thread local bindings, or can they be root-bound only when compiling clojure?
  4. will add *debug* binding to clojure.main/with-bindings. Anywhere else?
  5. build.xml should not have to change – system properties will flow through (and build.xml may not be around much longer anyway)
  6. once we agree on the approach, I will ping maven plugin and lein owners so that they flow the setting through
  7. better assertion messages will be a separate ticket
  8. what is the interaction between *debug* and *unchecked-math*? Change checks to (and *unchecked-math* (not *debug*))}?
Comment by Rich Hickey [ 08/Dec/10 11:00 AM ]

#3 - root bound only
#4 - should not be in with-bindings for same reason as #3 - we don't want people to set! *debug* nor *assert*
#8 - yes, wrapping that in a helper fn

#6 - my biggest reservation is that this isn't yet informed by maven best practices

Comment by Stuart Sierra [ 08/Dec/10 2:09 PM ]

System properties can be passed through Maven, so I do not anticipate this being a problem.

However, I would prefer *assert* to remain true by default.

Comment by Chas Emerick [ 09/Dec/10 7:19 AM ]

SS is correct about this approach not posing any issue for Maven. In addition, the build could easily be set up to always emit two jars, one "normal", one "debug".

I'd suggest that, while clojure.debug might have broad effect, additional properties should be available to provide fine-grained control over each of the additional "debug"-related parameterizations that might become available in the future.


I'd like to raise a couple of potentially tangential concerns (after now thinking about assertions for a bit in the above context), some or all of which may simply be a result of my lack of understanding in various areas.

Looking at where assert is used in core.clj (only two places AFAICT: validating arguments to derive and checking pre- and post-conditions in fn), it would seem unwise to make it false by default. i.e. non-Named values would be able to get into hierarchies, and pre- and post-conditions would simply be ignored.

It's my understanding that assertions (talking here of the JVM construct, from which Clojure reuses AssertionError) should not be used to validate arguments to public API functions, or used to validate any aspect of a function's normal operation (i.e. "where not to use assertions"). That would imply that derive should throw IllegalArugmentException when necessary, and fn pre- and post-conditions should perhaps throw IllegalStateException – or, in any case, something other than AssertionError via assert. This would match up far better with most functions in core using assert-args rather than assert, the former throwing IllegalArgumentException rather than AssertionError.

That leads me to the question: is assert (and *assert*) intended to be a Clojure construct, or a quasi-interop form?

If the former, then it can roughly have whatever semantics we want, but then it seems like it should not be throwing AssertionError.

If the latter, then AssertionError is appropriate on the JVM, but then we need to take care that assertions can be enabled and disabled at runtime (without having to switch around different builds of Clojure), ideally using the host-defined switches (e.g. -ea and friends) and likely not anything like *assert*. I don't know if this is possible or practical at this point (I presume this would require nontrivial compiler changes).


Hopefully the above is not water under the bridge at this point. Thank you in advance for your forbearance.

Comment by Rich Hickey [ 09/Dec/10 8:08 AM ]

Thanks for the useful input Chas. Nothing is concluded yet. I think we should step back and look at the objective here, before moving forward with a solution. Being a dynamic language, there are many things we might want to validate about our programs, where the cost of checking is something we are unwilling to pay in production.

Being a macro, assert has the nice property that, should *assert* not be true during compilation, it generates nil, no conditional test at all. Thus right now it is more like static conditional compilation.

Java assert does have runtime toggle-ability, via -ea as you say. I haven't looked at the bytecode generated for Java asserts, but it might be possible for Clojure assert to do something similar, if the runtime overhead is negligible. It is quite likely that HotSpot has special code for eliding the assertion bytecode given a single check of some flag. I'm just not sure that flag is Class.desiredAssertionStatus.

Whether this turns into changes in assert or pre/post conditions, best practices etc is orthogonal and derived. Currently we don't have a facility to run without the checks. We need to choose between making them disappear during compilation (debug build) or runtime (track -ea) or both. Then we can look at how that is reflected in assert/pre-post and re-examine existing use of both. The "where not to use assertions" doc deals with them categorically, but in not considering their cost, seems unrealistic IMO.

I'd appreciate it if someone could look into how assert is generated and optimized by Java itself.

Comment by Chas Emerick [ 09/Dec/10 5:04 PM ]

Bytecode issues continue to be above my pay grade, unfortunately…

A few additional thoughts in response that you may or may not be juggling already:

assert being a macro makes total sense for what it's doing. Trouble is, "compile-time" is a tricky concept in Clojure: there's code-loading-time, AOT-compile-time, and transitively-AOT-compile-time. Given that, it's entirely possible for an application deployed to a production environment that contains a patchwork of code or no code produced by assert usages in various libraries and namespaces depending upon when those libs and ns' were loaded, AOT-compiled, or their dependents AOT-compiled, and the value of *assert* at each of those times. Of course, this is the case for all such macros whose results are dependent upon context-dependent state (I think this was a big issue with clojure.contrib.logging, making it only usable with log4j for a while).

What's really attractive about the JVM assertion mechanism is that it can be parameterized for a given runtime on a per-package basis, if desired. Reimplementing that concept so that assert can be *ns*-sensitive seems like it'd be straightforward, but the compile-time complexity already mentioned remains, and the idea of having two independently-controlled assertion facilities doesn't sound fun.

I know nearly nothing about the CLR, but it would appear that it doesn't provide for anything like runtime-controllable assertions.

Comment by Stuart Halloway [ 29/Dec/10 3:17 PM ]

The best (dated) evidence I could find says that the compiler sets a special class static final field $assertionsDisabled based on the return of desiredAssertionStatus. HotSpot doesn't do anything special with this, dead code elimination simply makes it go away. The code indeed compiles this way:

11: getstatic #6; //Field $assertionsDisabled:Z
14: ifne 33
17: lload_1
18: lconst_0
19: lcmp
20: ifeq 33
23: new #7; //class java/lang/AssertionError
26: dup
27: ldc #8; //String X should be zero
29: invokespecial #9; //Method java/lang/AssertionError."<init>":(Ljava/lang/Object;)V
32: athrow

Even if we were 100% sure that assertion removal was total, I would still vote for a separate Clojure-level switch, for the following reasons:

  1. I have a real and pressing need to disable some assertions, and I don't need the Java interop at all. Arguably others will be in the same boat.
  2. there will be multiple debugging facilities over time, and having a top-level debug switch is convenient for Clojure users.
  3. Java dis/enabling via command line flags is still possible as a separate feature. We could add this later as a (small) breaking change to our assert, or have a separate java-assert interop form. I am on the fence about which way to go here.
  4. I believe it is perfectly fine to throw an AssertionError from a non-Java-assertion-form. We don't believe in a world of a static exception hierarchy, and an assertion in production is a critical failure no matter what you call it. Even Scala does it http://daily-scala.blogspot.com/2010/03/assert-require-assume.html

Rich: awaiting your blessing to move forward on this.

Comment by Rich Hickey [ 07/Jan/11 9:42 AM ]

The compiler sets $assertionsDisabled when, in static init code? Is there special support in the classloader for it? Is there a link to the best dated evidence you found?

Comment by Stuart Halloway [ 07/Jan/11 9:51 AM ]
  1. Yes, in static init code
  2. There is no special support in the classloader, per Brian Goetz (private correspondence) last week. But dead code elimination is great: "The run-time cost of disabled assertions should indeed be zero for compiled code"
Comment by Stuart Halloway [ 07/Jan/11 9:57 AM ]

Link: Google "java assert shirazi". (Not posting link because I can't tell in 10 secs whether it includes my session information.)

Comment by Alexander Kiel [ 14/Mar/13 1:28 PM ]

Is there anything new on this issue? I also look for a convenient way to disable assertions in production.

Comment by Michał Łopuszyński [ 11/Oct/13 4:21 AM ]

I am also interested in any news on this issue.
Convenient way to enable/disable assertions at runtime (preferably via -ea/-da options) would be a great feature!

Comment by Michał Łopuszyński [ 23/Oct/13 3:12 AM ]

Btw. there is a library for runtime-toggable assertions available via clojars
https://github.com/pjstadig/assertions
This helped me a great deal.





[CLJ-291] (take-nth 0 coll) redux... Created: 08/Apr/10  Updated: 27/Jul/13

Status: In Progress
Project: Clojure
Component/s: None
Affects Version/s: None
Fix Version/s: Backlog

Type: Enhancement Priority: Minor
Reporter: Assembla Importer Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None

Approval: Vetted

 Description   

I dont seem to be able make the old ticket uninvalid so here goes
(take-nth 0 coll) causes (at least on Solaris) infinite space and time consumption
It's not a printout error as the following code causes the problem too

(let [j 0
firstprod (apply * (doall (map #(- 1 %) (take-nth j (:props mix)))))]) ; from my parameter update function

I used jvisualvm and the jvm is doing some RNI call - no clojure code is running at all
If left alone it will crash the jvm with all heap space consumed
0 is an InvalidArgument for take-nth
I wouldnt mind if it produced an infinite lazy sequence of nils even though thats wrong
It doesnt do this though it actively destroys the JVM
Its a bug nasty destructive and it took me half a day to figure out what was going on
please let someone fix it!



 Comments   
Comment by Assembla Importer [ 17/Oct/10 9:47 AM ]

Converted from http://www.assembla.com/spaces/clojure/tickets/291
Attachments:
fixbug291.diff - https://www.assembla.com/spaces/clojure/documents/dfNhoS2Cir3543eJe5cbLA/download/dfNhoS2Cir3543eJe5cbLA

Comment by Assembla Importer [ 17/Oct/10 9:47 AM ]

bhurt said: Before this bug gets marked as invalid as well, let me point out that the problem here is that (take-nth 0 any-list) is a meaningless construction- the only question is what to do when this happens. IMHO, the correct behavior is to throw an exception.

Comment by Assembla Importer [ 17/Oct/10 9:47 AM ]

ataggart said: [file:dfNhoS2Cir3543eJe5cbLA]: throws IllegalArgumentException on negative step size

Comment by Stuart Halloway [ 29/Oct/10 10:36 AM ]

Does calling (take-nth 0 ...) cause the problem, or only realizing the result?

Comment by Chouser [ 29/Oct/10 11:06 AM ]

I'm not seeing a problem. Calling take-nth and even partially consuming the seq it returns works fine for me:

(take 5 (take-nth 0 [1 2 3]))
;=> (1 1 1 1 1)

Note however that it is returning an infinite lazy seq. The example in the issue description seems to include essentially (doall <infinite-lazy-seq>) which does blow the heap:

(doall (range))

This issue still strikes me as invalid.

Comment by Rich Hickey [ 29/Oct/10 11:06 AM ]

(take-nth 0 ...) returns an infinite sequence of the first item:

(take 12 (take-nth 0 [1 2 3]))
=> (1 1 1 1 1 1 1 1 1 1 1 1)

Is something other than this happening on Solaris?





[CLJ-326] add :as-of option to refer Created: 30/Apr/10  Updated: 26/Jul/13

Status: In Progress
Project: Clojure
Component/s: None
Affects Version/s: None
Fix Version/s: Backlog

Type: Enhancement Priority: Minor
Reporter: Christophe Grand Assignee: Christophe Grand
Resolution: Unresolved Votes: 0
Labels: None

Approval: Vetted

 Description   

Discussed here: http://groups.google.com/group/clojure-dev/msg/74af612909dcbe56

:as-of allows library authors to specify a known subset of vars to refer from clojure (or any other library which would use :added metadata).

(ns foo (:refer-clojure :as-of "1.1")) is equivalent to (ns foo (:refer-clojure :only [public-documented-vars-which-already-existed-in-1.1]))



 Comments   
Comment by Assembla Importer [ 24/Aug/10 10:19 AM ]

Converted from http://www.assembla.com/spaces/clojure/tickets/326
Attachments:
add-as-of-to-refer.patch - https://www.assembla.com/spaces/clojure/documents/a8SumUvcOr37SmeJe5cbLA/download/a8SumUvcOr37SmeJe5cbLA

Comment by Assembla Importer [ 24/Aug/10 10:19 AM ]

cgrand said: [file:a8SumUvcOr37SmeJe5cbLA]: requires application of #325

Comment by Assembla Importer [ 24/Aug/10 10:19 AM ]

richhickey said: Do we still need this?





[CLJ-346] (pprint-newline :fill) is not handled correctly Created: 12/May/10  Updated: 03/Sep/13

Status: Open
Project: Clojure
Component/s: None
Affects Version/s: None
Fix Version/s: Backlog

Type: Defect Priority: Minor
Reporter: Assembla Importer Assignee: Tom Faulhaber
Resolution: Unresolved Votes: 0
Labels: print

Approval: Vetted
Waiting On: Tom Faulhaber

 Description   

Filled pretty printing (where we try to fit as many elements on a line as possible) is being too aggressive as we can see when we try to print the following array:

user> (binding [*print-right-margin* 20] (pprint (int-array (range 10))))

Produces:

[0,
1,
2,
3,
4, 5, 6, 7, 8, 9]

Rather than

[0, 1, 2, 3, 4,
5, 6, 7, 8, 9]

or something like that. (I haven't worked through the exact correct representation for this case).

We currently only use :fill style newlines for native java arrays.



 Comments   
Comment by Assembla Importer [ 24/Aug/10 8:01 AM ]

Converted from http://www.assembla.com/spaces/clojure/tickets/346
Attachments:
0347-pprint-update-2.diff - https://www.assembla.com/spaces/clojure/documents/diLxv6y4Sr35GVeJe5cbLr/download/diLxv6y4Sr35GVeJe5cbLr

Comment by Assembla Importer [ 24/Aug/10 8:01 AM ]

stu said: [file:diLxv6y4Sr35GVeJe5cbLr]

Comment by Assembla Importer [ 24/Aug/10 8:01 AM ]

stu said: The second patch includes the first, and adds another test.

Comment by Assembla Importer [ 24/Aug/10 8:01 AM ]

tomfaulhaber said: This patch was attached to the wrong bug. It should be attached to bug #347. There is no fix for this bug yet.

Comment by Rich Hickey [ 05/Nov/10 8:07 AM ]

Is this current?

Comment by Stuart Halloway [ 29/Nov/10 8:48 PM ]

Tom, this patch doesn't apply, and I am not sure why. Can you take a look?





[CLJ-348] reify allows use of qualified name as method parameter Created: 13/May/10  Updated: 26/Jul/13

Status: Open
Project: Clojure
Component/s: None
Affects Version/s: None
Fix Version/s: Backlog

Type: Defect Priority: Minor
Reporter: Assembla Importer Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None

Approval: Vetted

 Description   

This should complain about using a fully-qualified name as a parameter:

(defmacro lookup []
`(reify clojure.lang.ILookup
(valAt [_ key])))

Instead it simply ignores that parameter in the method body in favour of clojure.core/key.



 Comments   
Comment by Assembla Importer [ 24/Aug/10 8:03 AM ]

Converted from http://www.assembla.com/spaces/clojure/tickets/348
Attachments:
0001-Add-a-test-for-348-reify-shouldn-t-accept-qualified-.patch - https://www.assembla.com/spaces/clojure/documents/d2xUJIxTyr36fseJe5cbLA/download/d2xUJIxTyr36fseJe5cbLA

Comment by Assembla Importer [ 24/Aug/10 8:03 AM ]

technomancy said: [file:d2xUJIxTyr36fseJe5cbLA]: A test to expose the unwanted behaviour

Comment by Assembla Importer [ 24/Aug/10 8:03 AM ]

richhickey said: I'm not sure the bug is what you say it is, or the resolution should be what you suggest. The true problem is the resolution of key when qualified. Another possibility is to ignore the qualifier there.

Comment by Assembla Importer [ 24/Aug/10 8:03 AM ]

technomancy said: Interesting. So it's not appropriate to require auto-gensym here? Why are the rules different for reify methods vs proxy methods?

> (defmacro lookup []
`(proxy [clojure.lang.ILookup] []
(valAt [key] key)))
> (lookup)

Can't use qualified name as parameter: clojure.core/key
[Thrown class java.lang.Exception]





[CLJ-731] Create macro to variadically unroll a combinator function definition Created: 26/Jan/11  Updated: 26/Jul/13

Status: Open
Project: Clojure
Component/s: None
Affects Version/s: None
Fix Version/s: Backlog

Type: Enhancement Priority: Minor
Reporter: Fogus Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None

Approval: Vetted

 Description   

Clojure contains a set of combinators that are implemented in a similar, but slightly different way. That is, they are implemented as a complete set of variadic overloads on both the call-side and also on the functions that they return. Visually, they all tend to look something like:

(defn foo
  ([f]
     (fn
       ([] do stuff...)
       ([x] do stuff...)
       ([x y] do stuff...)
       ([x y z] do stuff...)
       ([x y z & args] do variadic stuff...)))
  ([f1 f2]
     (fn
       ([] do stuff...)
       ([x] do stuff...)
       ([x y] do stuff...)
       ([x y z] do stuff...)
       ([x y z & args] do variadic stuff...)))
  ([f1 f2 f3]
     (fn
       ([] do stuff...)
       ([x] do stuff...)
       ([x y] do stuff...)
       ([x y z] do stuff...)
       ([x y z & args] do variadic stuff...)))
  ([f1 f2 f3 & fs]
     (fn
       ([] do stuff...)
       ([x] do stuff...)
       ([x y] do stuff...)
       ([x y z] do stuff...)
       ([x y z & args] do variadic stuff...))))

To build this type of function for each combinator is tedious and error-prone.

There must be a way to implement a macro that takes a "specification" of a combinator including:

1. name
2. docstring
3. do stuff template
4. do variadic stuff template

And builds something like the function foo above. This macro should be able to implement the current batch of combinators (assuming that http://dev.clojure.org/jira/browse/CLJ-730 is completed first for the sake of verification).



 Comments   
Comment by Stuart Halloway [ 28/Jan/11 9:03 AM ]

This seems useful. Rich, would you accept a patch?

Comment by Stuart Halloway [ 28/Jan/11 9:40 AM ]

Nevermind, just saw that Rich already suggested this on the dev list. Patch away.





[CLJ-823] Piping seque into seque can deadlock Created: 03/Aug/11  Updated: 18/Apr/14

Status: Open
Project: Clojure
Component/s: None
Affects Version/s: Release 1.3
Fix Version/s: Release 1.7

Type: Defect Priority: Minor
Reporter: Greg Chapman Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: None
Environment:

Windows 7; JVM 1.6; Clojure 1.3 beta 1


Approval: Vetted

 Description   

I'm not sure if this is a supported scenario, but the following deadlocks in Clojure 1.3:

(let [xs (seque (range 150000))
ys (seque (filter odd? xs))]
(apply + ys))

As I understand it, the problem is that ys' fill takes place on an agent thread, so when it calls xs' drain, the (send-off agt fill) does not immediately trigger xs' fill, but is instead put on the nested list to be performed when ys' agent returns. Unfortunately, ys' fill will eventually block trying to take from xs, and so it never returns and the pending send-offs are never sent. Wrapping the send-off in drain to:

(future (send-off agt fill))

is a simple (probably not optimal) way to fix the deadlock.



 Comments   
Comment by Peter Monks [ 07/Jan/13 3:43 PM ]

Reproduced on 1.4.0 and 1.5.0-RC1 as well, albeit with this example:

(seque 3 (seque 3 (range 10)))

Comment by Stuart Halloway [ 30/Mar/13 9:16 AM ]

release-pending-sends?





Generated at Tue Jul 22 12:35:32 CDT 2014 using JIRA 4.4#649-r158309.