Clojure JIRAhttp://dev.clojure.org/jira/secure/IssueNavigator.jspa?reset=true&jqlQuery=project+%3D+CLJ+AND+status+in+%28Open%2C+%22In+Progress%22%2C+Reopened%29+AND+fixVersion+IS+NULL+and+Approval+%3D+Vetted+ORDER+BY+type+ASC%2C+priority+DESC%2C+key+DESC
An XML representation of a search requesten-us4.464925-07-2011[CLJ-1610] Unrolled small mapshttp://dev.clojure.org/jira/browse/CLJ-1610
Clojure<p>Placeholder for unrolled small maps enhancement (companion for vectors at <a href="http://dev.clojure.org/jira/browse/CLJ-1517" title="Unrolled small vectors">CLJ-1517</a>).</p>CLJ-1610Unrolled small mapsEnhancementCriticalOpenUnresolvedZach TellmanAlex MillercollectionsMon, 8 Dec 2014 13:09:29 -0600Fri, 31 Jul 2015 08:00:12 -0500Release 1.666<p>Is there an expectation that these would perform better that PersistentArrayMap?</p><p>Yes, in some cases significantly so, for three reasons (in rough order of importance):</p>
<ul>
<li>positional constructors, without any need for array instantiation/population</li>
<li>short-circuiting equality checks using hash comparisons</li>
<li>no iteration on any operation</li>
</ul>
<p>There are a series of benchmarks at <a href="https://github.com/ztellman/cambrian-collections/blob/master/test/cambrian_collections/map_test.clj#L64-L148">https://github.com/ztellman/cambrian-collections/blob/master/test/cambrian_collections/map_test.clj#L64-L148</a>, which compare operations against maps with both keywords (which don't benefit from the hash comparisons) and symbols (which do). The 7-entry map cases cause the unrolled maps to overflow, so they only exist to test the overflow mechanism.</p>
<p>I've run the benchmark suite on my laptop, and the results are at <a href="https://gist.github.com/ztellman/961001e1a77e4f76ee1d">https://gist.github.com/ztellman/961001e1a77e4f76ee1d</a>. Some notable results:</p>
<ul>
<li>non-keyword lookups on 5-element maps are 2x faster (hash checks): <a href="https://gist.github.com/ztellman/961001e1a77e4f76ee1d#file-gistfile1-txt-L169-L191">https://gist.github.com/ztellman/961001e1a77e4f76ee1d#file-gistfile1-txt-L169-L191</a></li>
<li>creation for 1-element maps is 7-10x faster, and for 3-element maps is 4-8x faster (positional constructors): <a href="https://gist.github.com/ztellman/961001e1a77e4f76ee1d#file-gistfile1-txt-L2178-L2307">https://gist.github.com/ztellman/961001e1a77e4f76ee1d#file-gistfile1-txt-L2178-L2307</a></li>
<li>creation of 5-element maps via `into` is 1.5x-2.5x faster: <a href="https://gist.github.com/ztellman/961001e1a77e4f76ee1d#file-gistfile1-txt-L1673-L1719">https://gist.github.com/ztellman/961001e1a77e4f76ee1d#file-gistfile1-txt-L1673-L1719</a></li>
</ul>
<p>The rest of the benchmarks are marginally faster due to unrolling, but most of the performance benefits are from the above behaviors. In a less synthetic benchmark, I found that Cheshire JSON decoding (which is 33% JSON lexing and 66% data structure building) was sped up roughly 30-40%. </p>ApprovalVettedGlobal Rank[CLJ-1553] Parallel transducehttp://dev.clojure.org/jira/browse/CLJ-1553
Clojure<p>Consider how to create a parallel path for transducers, similar to reducers fold.</p>CLJ-1553Parallel transduceEnhancementMajorOpenUnresolvedUnassignedAlex MillertransducersTue, 7 Oct 2014 12:58:07 -0500Sun, 12 Jul 2015 21:33:44 -0500Release 1.754ApprovalVettedGlobal Rank[CLJ-1552] Consider kv support for transducers (similar to reducers fold)http://dev.clojure.org/jira/browse/CLJ-1552
Clojure<p>In reducers, fold over a map has special support for kv. Consider whether/how to add this for transducers.</p>CLJ-1552Consider kv support for transducers (similar to reducers fold)EnhancementMajorOpenUnresolvedUnassignedAlex MillertransducersTue, 7 Oct 2014 12:56:59 -0500Sun, 12 Jul 2015 21:33:57 -0500Release 1.702<p>We don't have a JIRA "unvote" feature, but I'd like to register my vote <b>against</b> this proposed enhancement. As a heavy user of clojure.core.reducers, I consider the switch to k-v semantics when reducing a map to be a significant mis-feature. As only an initial transformation function applied directly to a map is able to receive the k-v semantics (a limitation I can’t see how would not carry over to transducers), this behavior crops up most frequently when re-ordering operations and discovering that an intermediate map has now caused an airity error somewhere in the middle of a chain of threaded transformations. I’ve never found cause to invoke it intentionally.</p>ApprovalVettedGlobal Rank[CLJ-1551] Consider transducer support for primitiveshttp://dev.clojure.org/jira/browse/CLJ-1551
Clojure<p>Need to consider how we can support primitives for transducers. In particular it may be that IFn needs overloading for L/D in addition to O. </p>CLJ-1551Consider transducer support for primitivesEnhancementMajorOpenUnresolvedUnassignedAlex MillertransducersTue, 7 Oct 2014 12:55:19 -0500Sun, 12 Jul 2015 21:34:17 -0500Release 1.702ApprovalVettedGlobal Rank[CLJ-1130] When unable to match a static method, report arity caller was looking for, avoid misleading field errorhttp://dev.clojure.org/jira/browse/CLJ-1130
Clojure<p>Incorrectly invoking a static method with 0 parameters yields a NoSuchFieldException:</p>
<div class="code panel" style="border-width: 1px;"><div class="codeContent panelContent">
<pre class="code-java">user=&gt; (<span class="code-object">Long</span>/parseLong)
CompilerException java.lang.NoSuchFieldException: parseLong, compiling:(NO_SOURCE_PATH:1:1)
user=&gt; (<span class="code-object">Long</span>/parseLong <span class="code-quote">"5"</span> 10 3)
CompilerException java.lang.IllegalArgumentException: No matching method: parseLong, compiling:(NO_SOURCE_PATH:2:1)</pre>
</div></div>
<p><b>Approach:</b> Error reporting enhanced to report desired arg count and to avoid a field exception confusion if 0 arg method.</p>
<p>user=&gt; (Long/parseLong)<br/>
CompilerException java.lang.IllegalArgumentException: No matching method parseLong found taking 0 args for class java.lang.Long, compiling:(NO_SOURCE_PATH:1:1)<br/>
user=&gt; (Long/parseLong "5" 10 3)<br/>
CompilerException java.lang.IllegalArgumentException: No matching method parseLong found taking 3 args for class java.lang.Long, compiling:(NO_SOURCE_PATH:2:1)</p>
<p><b>Patch:</b> clj-1130-v5.diff</p>
<p><b>Screened by:</b> Alex Miller</p>CLJ-1130When unable to match a static method, report arity caller was looking for, avoid misleading field errorEnhancementMajorOpenUnresolvedUnassignedHoward Lewis ShiperrormsgsftMon, 17 Dec 2012 17:57:44 -0600Tue, 21 Jul 2015 07:51:11 -0500Release 1.412<p>It looks like it's first trying to resolve a field by name, since field access with <tt>/</tt> is legal. For example:</p>
<p><tt>user=&gt; (Integer/parseInt)</tt><br/>
<tt>CompilerException java.lang.NoSuchFieldException: parseInt, compiling:(NO_SOURCE_PATH:1)</tt></p>
<p><tt>user=&gt; (Integer/MAX_VALUE)</tt><br/>
<tt>2147483647</tt></p>
<p>Would trying to resolve a method before a field fix this?</p><p>Similarities to <a href="http://dev.clojure.org/jira/browse/CLJ-1248" title="Show type information in reflection warning messages when available"><del>CLJ-1248</del></a> (there a warning, here an error). </p><p>Patch clj-1130-v1.txt changes the error message in a situation when one attempts to invoke a static method with no args, and there is no such 0-arg static method. The message now says that there is no such method with that name and 0 args, rather than that there is no such static field with that name.</p><p>I updated the patch to simplify it a bit but more importantly to remove the check by exception and instead use the Reflector method that can answer this question. </p><p>Alex, thank you for the improvements to the code. It looks better to me.</p><p>due to indentation changes, this patch appears to touch much more than it probably does, making it difficult to approve.</p><p>Any suggestions on what can be done to make progress here? Would it help to attach a patch made with "-w" option to ignore lines that differ only in whitespace? Provide git diff command line options that do this, after the patch is applied to your local workspace? Make a patch that leaves the indentation 'incorrect' after the change (involuntary shudder)?</p><p>The indentation has intentionally changed because the if/else structure has changed. I don't think making the patch incorrect to reduce changes is a good idea. <img class="emoticon" src="http://dev.clojure.org/jira/images/icons/emoticons/smile.gif" height="20" width="20" align="absmiddle" alt="" border="0"/> </p><p>Well, the 'incorrect' was in quotes because I was asking about a proposed patch that had the correct logic, but misleading indentation. Agreed it isn't a good idea, hence the shudder. I'm just brainstorming ideas to make the patch less difficult to approve.</p><p>At some point, you may need to bite the bullet and reformat some of the Clojure code .... Compiler.java had a crazy mix of tabs, spaces, and just completely wrong stuff.</p><p>Re-marking screened. Not sure what else to do.</p><p>clj-1130-v2-ignore-ws.diff is identical to clj-1130-v2.diff, except it was produced with a command that ignores differences in a line due only to whitespace, i.e.: 'git format-patch master --stdout -w &gt; clj-1130-v2-ignore-ws.diff'</p>
<p>It is not intended as the patch to be applied. It is only intended to make it easier to see that many of the lines in clj-1130-v2.diff are truly only differences in indentation.</p><p>Thanks Andy...</p><p>This patch ignores the fact that method is checked for first above:</p>
<div class="code panel" style="border-width: 1px;"><div class="codeContent panelContent">
<pre class="code-java"><span class="code-keyword">if</span>(c != <span class="code-keyword">null</span>)
maybeField = Reflector.getMethods(c, 0, munge(sym.name), <span class="code-keyword">true</span>).size() == 0;</pre>
</div></div>
<p>Which is why the field code is unconditional. I'm fine with making errors better, but changing logic as well deserves more scrutiny.</p><p>This patch is intentionally trying to avoid calling StaticFieldExpr in the field code as that is where the <tt>(Long/parseLong)</tt> case (erroneously calling an n-arity static method with 0 args) will throw a field-oriented exception instead of a method-oriented exception. By adding the extra check here, this case falls through into the method case and throws later on calling StaticMethodExpr instead.</p>
<p>The early check is a check for methods of the specified arity. The later check is for the existence of a field of matching name. Combined, they lead to a better error message. </p>
<p>However, another alternative is to set maybeField in the first check based on field existence, not on invocation arity. That just improves the maybeField informaiton and the existing code then naturally throws the correct exception (and the patch is much simpler). </p>
<p>The similar case for calling n-arity instance methods with 0-arity has the same problem for the same reason:</p>
<div class="code panel" style="border-width: 1px;"><div class="codeContent panelContent">
<pre class="code-java">user=&gt; (.setTime (java.util.Date.))
IllegalArgumentException No matching field found: setTime <span class="code-keyword">for</span> class java.util.Date clojure.lang.Reflector.getInstanceField (Reflector.java:271)</pre>
</div></div>
<p>Thus we can also adjust the other call that sets maybeField (which now is much less maybe).</p>
<p>I will attach a patch that covers these cases and update the ticket for someone to screen.</p><p>Screened. The patch clj-1130-v3.diff works as advertised.</p>
<p>This patch only improves error messages for cases when the type of the<br/>
target object is known to the compiler. In reflective calls, the error<br/>
messages are still the same.</p>
<p>Example, after this patch, given these definitions:</p>
<div class="code panel" style="border-width: 1px;"><div class="codeContent panelContent">
<pre class="code-java">(def v 42)
(defn untagged-f [] 42)
(defn ^<span class="code-object">Long</span> tagged-f [] 42)</pre>
</div></div>
<p>The following expressions produce new error messages:</p>
<div class="code panel" style="border-width: 1px;"><div class="codeContent panelContent">
<pre class="code-java">(.foo v 1)
;; IllegalArgumentException No matching method found: foo taking 1 args
;; <span class="code-keyword">for</span> class java.lang.<span class="code-object">Long</span> clojure.lang.Reflector.invokeMatchingMethod
;; (Reflector.java:53)
(.foo (tagged-f))
;; IllegalArgumentException No matching method found: foo taking 0 args
;; <span class="code-keyword">for</span> class java.lang.<span class="code-object">Long</span> clojure.lang.Reflector.invokeMatchingMethod
;; (Reflector.java:53)</pre>
</div></div>
<p>These expressions still use the old error messages:</p>
<div class="code panel" style="border-width: 1px;"><div class="codeContent panelContent">
<pre class="code-java">(.foo v)
;; IllegalArgumentException No matching field found: foo <span class="code-keyword">for</span> class
;; java.lang.<span class="code-object">Long</span> clojure.lang.Reflector.getInstanceField
;; (Reflector.java:271)
(.foo (untagged-f))
;; IllegalArgumentException No matching field found: foo <span class="code-keyword">for</span> class
;; java.lang.<span class="code-object">Long</span> clojure.lang.Reflector.getInstanceField
;; (Reflector.java:271)</pre>
</div></div><p>Changing the logic to get a different error message is something that needs to be done with great care. This now seems to prefer fields over methods, changing the semantics.</p><p>v4 patch simply enhances error messaages</p><p>clj-1130-v4.diff has the same patch repeated twice in the file. clj-1130-v5.diff is identical, except deleting the redundant copy.</p>ApprovalVettedGlobal RankPatchCode and Test[CLJ-1005] Use transient map in zipmaphttp://dev.clojure.org/jira/browse/CLJ-1005
Clojure<p>#'zipmap constructs a map without transients, where transients could improve performance.</p>
<p><b>Approach:</b> Use a transient map internally, along with iterators for the keys and values. A persistent map is returned as before. The definition is also moved so that it resides below that of #'transient.</p>
<p><b>Performance:</b></p>
<p>(def xs (range 16384))<br/>
(def ys (range 16)) </p>
<table class='confluenceTable'><tbody>
<tr>
<th class='confluenceTh'> expression </th>
<th class='confluenceTh'> 1.7.0-beta3 </th>
<th class='confluenceTh'> +patch </th>
<th class='confluenceTh'>&nbsp;</th>
</tr>
<tr>
<td class='confluenceTd'> (zipmap xs xs) </td>
<td class='confluenceTd'> 4.50 ms </td>
<td class='confluenceTd'> 2.12 ms </td>
<td class='confluenceTd'> large map</td>
</tr>
<tr>
<td class='confluenceTd'> (zipmap ys ys) </td>
<td class='confluenceTd'> 2.75 us </td>
<td class='confluenceTd'> 2.07 us </td>
<td class='confluenceTd'> small map</td>
</tr>
</tbody></table>
<p><b>Patch:</b> <a href="http://dev.clojure.org/jira/browse/CLJ-1005" title="Use transient map in zipmap">CLJ-1005</a>-zipmap-iterators.patch</p>
<p><b>Screened by:</b> Alex Miller</p>CLJ-1005Use transient map in zipmapEnhancementMajorOpenUnresolvedUnassignedMichał MarczykftperformanceWed, 30 May 2012 02:55:43 -0500Sun, 19 Jul 2015 07:39:54 -0500Release 1.5136<p>Why is the old implementation left and commented out? If we are going to move to a new implementation, the old one should be removed.</p><p>As mentioned in the ticket description, the previously attached patch follows the pattern of into whose non-transient-enabled definition is left in core.clj with a #_ in front &#8211; I wasn't sure if that's something desirable in all cases.</p>
<p>Here's a new patch with the old impl removed.</p><p>Thanks for the updated patch, Michal. Sorry to raise such a minor issue, but would you mind using a different name for the updated patch? I know JIRA can handle multiple attached files with the same name, but my prescreening code isn't quite that talented yet, and it can lead to confusion when discussing patches.</p><p>Thanks for the heads-up, Andy! I've reattached the new patch under a new name.</p><p>Presumptuously changing Approval from Incomplete back to None after the Michal's updated patch was added, addressing the reason the ticket was marked incomplete.</p><p>The patch looks good and applies cleanly. Are there additional tests that we should run to verify that this is providing the improvement we think it is. Also, is there a discussion somewhere that started this ticket? There isn't a lot of context here.</p><p>Hi Aaron,</p>
<p>Thanks for looking into this!</p>
<p>From what I've been able to observe, this change hugely improves <tt>zipmap</tt> times for large maps. For small maps, there is a small improvement. Here are two basic Criterium benchmarks (<tt>transient-zipmap</tt> defined at the REPL as in the patch):</p>
<div class="preformatted panel" style="border-width: 1px;"><div class="preformattedContent panelContent">
<pre>;;; large map
user=&gt; (def xs (range 16384))
#'user/xs
user=&gt; (last xs)
16383
user=&gt; (c/bench (zipmap xs xs))
Evaluation count : 13920 in 60 samples of 232 calls.
Execution time mean : 4.329635 ms
Execution time std-deviation : 77.791989 us
Execution time lower quantile : 4.215050 ms ( 2.5%)
Execution time upper quantile : 4.494120 ms (97.5%)
nil
user=&gt; (c/bench (transient-zipmap xs xs))
Evaluation count : 21180 in 60 samples of 353 calls.
Execution time mean : 2.818339 ms
Execution time std-deviation : 110.751493 us
Execution time lower quantile : 2.618971 ms ( 2.5%)
Execution time upper quantile : 3.025812 ms (97.5%)
Found 2 outliers in 60 samples (3.3333 %)
low-severe 2 (3.3333 %)
Variance from outliers : 25.4675 % Variance is moderately inflated by outliers
nil
;;; small map
user=&gt; (def ys (range 16))
#'user/ys
user=&gt; (last ys)
15
user=&gt; (c/bench (zipmap ys ys))
Evaluation count : 16639020 in 60 samples of 277317 calls.
Execution time mean : 3.803683 us
Execution time std-deviation : 88.431220 ns
Execution time lower quantile : 3.638146 us ( 2.5%)
Execution time upper quantile : 3.935160 us (97.5%)
nil
user=&gt; (c/bench (transient-zipmap ys ys))
Evaluation count : 18536880 in 60 samples of 308948 calls.
Execution time mean : 3.412992 us
Execution time std-deviation : 81.338284 ns
Execution time lower quantile : 3.303888 us ( 2.5%)
Execution time upper quantile : 3.545549 us (97.5%)
nil
</pre>
</div></div>
<p>Clearly the semantics are preserved provided transients satisfy their contract.</p>
<p>I think I might not have started a ggroup thread for this, sorry.</p><p>Patch 0001-Use-transient-map-in-zipmap.2.patch dated Aug 15 2012 does not apply cleanly to latest master after some commits were made to Clojure on Sep 3 2014.</p>
<p>I have not checked whether this patch is straightforward to update. See the section "Updating stale patches" at <a href="http://dev.clojure.org/display/community/Developing+Patches">http://dev.clojure.org/display/community/Developing+Patches</a> for suggestions on how to update patches.</p><p>Thanks, Andy. It was straightforward to update – an automatic rebase. Here's the updated patch.</p><p>New patch using clojure.lang.RT/iter, criterium shows &gt;30% more perf in the best case. Less alloc probably but I didn't measure. <a href="http://dev.clojure.org/jira/browse/CLJ-1499" title="Replace seq-based iterators with direct iterators for all non-seq collections that use SeqIterator"><del>CLJ-1499</del></a> (better iterators) is related</p>ApprovalVettedGlobal RankPatchCode[CLJ-992] `iterate` reducerhttp://dev.clojure.org/jira/browse/CLJ-992
Clojure<p>Added a reducer implementation mirroring clojure.core/iterate. </p>
<p><b>Patch:</b> iterate-reducer.patch</p>
<p><b>Screened by:</b> </p>CLJ-992`iterate` reducerEnhancementMajorOpenUnresolvedUnassignedAlan MalloyreducersThu, 10 May 2012 21:40:46 -0500Wed, 29 Apr 2015 10:38:49 -0500Release 1.502<p>Should I have made this implement Seqable as well? It wasn't clear to me, because as far as I could see this was the only function in clojure.core.reducers that's generating a brand-new sequence rather than transforming an existing one.</p><p>Previous version neglected to include the seed value of the iteration in the reduce.</p><p>Currying iterate seems useless, albeit not harmful. </p>
<p>While implementing repeat, I couldn't use currying. Because 1-arity is already reserved for infinite repeat (<span class="error">&#91;n x&#93;</span> and <span class="error">&#91;x&#93;</span>, not <span class="error">&#91;n x&#93;</span> and <span class="error">&#91;n&#93;</span> if currying)</p>
<p>How about we just support currying for functions where last param is reducible?</p><p>This new patch replaces the previous patch. As requested, I am splitting up the large issue <a href="http://dev.clojure.org/jira/browse/CLJ-993" title="`range` reducer">CLJ-993</a> into smaller tickets.</p>
<p>Does not depend on any of my other reducer patches, but there will probably be some minor merge conflicts unless it is merged after <a href="http://dev.clojure.org/jira/browse/CLJ-1045" title="Generalize/refactor implementation of PersistentVector/coll-fold">CLJ-1045</a> and <a href="http://dev.clojure.org/jira/browse/CLJ-1046" title="Drop-while as a reducer">CLJ-1046</a>, and before <a href="http://dev.clojure.org/jira/browse/CLJ-993" title="`range` reducer">CLJ-993</a>.</p>ApprovalVettedGlobal RankPatchCode and Test[CLJ-322] Enhance AOT compilation process to emit classfiles only for explicitly-specified namespaceshttp://dev.clojure.org/jira/browse/CLJ-322
Clojure<p>Summary: still needs decision on implementation approach.</p>
<p>This was <a href="https://www.assembla.com/spaces/clojure-contrib/tickets/23">originally/erroneously reported</a> by Howard Lewis Ship in the clojure-contrib assembla:</p>
<div class="code panel" style="border-width: 1px;"><div class="codeContent panelContent">
<pre class="code-java">My build file specifies the namespaces to AOT compile but <span class="code-keyword">if</span> I include another namespace
(even from a JAR dependency) that is not AOT compiled, the other namespace will be compiled as well.
In my <span class="code-keyword">case</span>, I was using clojure-contrib's clojure.contrib.str-utils2 namespace, and I got a bunch of
clojure/contrib/str_utils2 classes in my output directory.
I think that the AOT compiler should NOT precompile any namespaces that are transitively reached,
only namespaces in the set specified by the command line are appropriate.
As currently coded, you will frequently find unwanted third-party dependencies in your output JARs;
further, <span class="code-keyword">if</span> multiple parties depend on the same JARs, <span class="code-keyword">this</span> could cause bloating and duplication in the
eventual runtime classpath.</pre>
</div></div>
<p>Having the option of shipping either all AOT-compiled classfiles or mixed source/AOT depending upon one's distribution requirements would make that phase of work with a clojure codebase significantly easier and less error-prone. The only question in my mind is what the default should be. We're all used to the current behaviour, but I'd guess that any nontrivial project where the form of the distributable matters (i.e. the source/AOT mix), providing as much control as possible by default makes the most sense. Given the tooling that most people are using, it's trivial (and common practice, IIUC) to provide a comprehensive list of namespaces one wishes to compile, so making that the default shouldn't be a hurdle to anyone. If an escape hatch is desired, a --transitive switch to clojure.lang.Compile could be added.</p>CLJ-322Enhance AOT compilation process to emit classfiles only for explicitly-specified namespacesEnhancementMajorIn ProgressUnresolvedUnassignedChas EmerickaotThu, 29 Apr 2010 00:20:00 -0500Thu, 8 Jan 2015 16:30:11 -06002519<p>Converted from <a href="http://www.assembla.com/spaces/clojure/tickets/322">http://www.assembla.com/spaces/clojure/tickets/322</a><br/>
Attachments:<br/>
aot-transitivity-option-compat-322.diff - <a href="https://www.assembla.com/spaces/clojure/documents/aI7Eu-HeGr35ImeJe5cbLA/download/aI7Eu-HeGr35ImeJe5cbLA">https://www.assembla.com/spaces/clojure/documents/aI7Eu-HeGr35ImeJe5cbLA/download/aI7Eu-HeGr35ImeJe5cbLA</a><br/>
aot-transitivity-option-322.diff - <a href="https://www.assembla.com/spaces/clojure/documents/aIWFiWHeGr35ImeJe5cbLA/download/aIWFiWHeGr35ImeJe5cbLA">https://www.assembla.com/spaces/clojure/documents/aIWFiWHeGr35ImeJe5cbLA/download/aIWFiWHeGr35ImeJe5cbLA</a></p><p>hlship said: I'd like to reinforce this. I've been doing research on Clojure build tools for an upcoming talk and all of them (Maven, Leiningen, Gradle) have the same problem: the AOT compile extends from the desired namespaces (such as one containing a :gen-class) to every reached namespace. This is going to cause a real ugliness when application A uses libraries B and C that both depend on library D (such as clojure-contrib) and B and C are thus both bloated with duplicate, unwanted AOT compiled classes from the library D.</p><p>cemerick said: This behaviour is an implementation detail of Clojure's AOT compilation process, and is orthogonal to any particular build tooling.</p>
<p>I am working on a patch that would provide a mechanism for such tooling to disable this default behaviour.</p><p>cemerick said: A first cut of a change to address this issue is here (<b>caution, work in progress!</b>):</p>
<p><a href="http://github.com/cemerick/clojure/commit/6f14e0790c0d283a7e44056adf1bb3f36bb16e0e">http://github.com/cemerick/clojure/commit/6f14e0790c0d283a7e44056adf1bb3f36bb16e0e</a></p>
<p>This makes available a new recognized system property, <tt>clojure.compiler.transitive</tt>, which defaults to <tt>true</tt>. When set/bound to false (i.e. <tt>-Dclojure.compiler.transitive=false</tt> when using <tt>clojure.lang.Compile</tt>), only the first loaded file (either the ns named in the call to <tt>compile</tt> or each of the namespaces named as arguments to <tt>clojure.lang.Compile</tt>) will have classfiles written to disk.</p>
<p>This means that this compilation invocation:</p>
<div class="code panel" style="border-width: 1px;"><div class="codeContent panelContent">
<pre class="code-java">java -cp &lt;your classpath&gt; -Dclojure.compiler.transitive=<span class="code-keyword">false</span> clojure.lang.Compile com.bar com.baz</pre>
</div></div>
<p>will generate classfiles only for <tt>com.bar</tt> and <tt>com.baz</tt>, but not for any of the namespaces or other files they load, require, or use.</p>
<hr />
<p>The only shortcoming of this WIP patch is that classfiles <b>are</b> still generated for proxy and gen-class classes defined outside of the explicitly-named namespaces. What I thought was a solution for this ended up breaking the loading of generated interfaces (as produced by defprotocol, etc).</p>
<p>I'll take a second look at this before the end of the week, but wanted to get this out there so as to get any comments people might have.</p><p>technomancy said: Looks good, but I'm having trouble getting it to work. I tried compiling from master of Chas's fork on github, but I still got the all the .class files generated with -Dclojure.compiler.transitive=false. It could be a quirk of the way I'm using ant to fork off processes though. Is it possible to set it using System/setProperty, or must it be given as a property on the command-line?</p><p>cemerick said: Bah, that's just bad documentation. :-/</p>
<p>The system property is only provided by <tt>clojure.lang.Compile</tt>; the value of it drives the binding of <tt>clojure.core/<b>transitive-compile</b></tt>, which has a root binding of true.</p>
<p>You should be able to configure the transitivity the same way you configure <tt><b>compile-path</b></tt> (system prop to <tt>clojure.lang.Compile</tt> or a direct binding when at the REPL, etc).</p>
<p>If not, ping me in irc or elsewhere.</p><p>meikelbrandmeyer said: I think, excluding parts 'load'ed is a little strong. I have some namespaces which load several parts from different files, but which belong to the same namespace. The most prominent example of such a case is clojure.core itself. I'm find with stopping require and use, but load is a bit too much, I'd say.</p><p>technomancy said: Chas: Thanks; will give that a go.</p>
<p>Meikel: Do people actually use load outside of clojure.core? I thought it was only used there because clojure.core is a "special" namespace where you want more vars to be available than can reasonably fit in a single file. Splitting up a namespace into several files is quite unadvisable otherwise.</p><p>technomancy said: I can confirm that this works for me modulo the proxy/gen-class issue that Chas mentioned. I would love to see this in Clojure 1.2; it would really clean up a lot of build-related issues.</p><p>meikelbrandmeyer said: I used it several times and this is the first time, I hear that it is unadvisable to do so. Even with a lower number of Vars in the namespace (c.c is here certainly exceptional) and might be of use to split several "sections" of code which belong to the same namespace but have different functionality. Whether to use a big comment in the source to indicate the section or split things into subfiles is a matter of taste. But it's a perfectly reasonable thing todo.</p>
<p>Another use case, where I use this (and c.c.lazy-xml, IIRC) is to conditionally load code depending on whether a dependency is available or not. Eg. vimclojure uses c.c.pprint and c.c.stacktrace/clj-stacktrace transparently depending on their availability.</p>
<p>There are perfectly legal uses of load. I don't see any "unadvisable" here.</p><p>cemerick said: Thanks, Meikel; I had forgotten about that use case, as I don't use load directly myself at all. I probably wouldn't say it's inadvisable, just mostly unnecessary. In any case, that's a good catch. It complicates things a bit, but we'll see what happens. I'm going to take another whack at resolving the proxy/gen-class case and narrowing the impact of nontransitivity to use and require later tonight.</p>
<p>I agree wholeheartedly that this should be in 1.2, assuming the technical bits work out. This has been an irritant for quite a long time. I actually believe that nontransitivity should be the <b>default</b> &#8211; no one wants or expects to have classfiles show up for dependencies show up when a project is AOT-compiled. I think the only negative impact would be whoever still fiddles with compilation at the REPL, and doesn't use maven or lein &#8211; and even then, it's just a matter of binding another var.</p><p>meikelbrandmeyer said: Then the var should be added to the default bindings in the clojure.main repl. Then it's set!-able like the other vars ��� <b>warn-on-reflection</b> and friends.</p><p>cemerick said: This is looking pretty good (<b>still WIP</b>):</p>
<p><a href="http://github.com/cemerick/clojure/commit/fedfb022ecef420a932b3d69c182ec7a8e5960a6">http://github.com/cemerick/clojure/commit/fedfb022ecef420a932b3d69c182ec7a8e5960a6</a></p>
<p>Thank you again for mentioning load, Meikel: it was very helpful in resolving the proxy/gen-class issue as well.</p>
<p>Just a single data point: the jar produced by the medium-sized project I've been using for testing the changes has shrunk from 1.8MB to less than 1MB. That's not the only reason this is a good change, but it's certainly a nice side-effect.</p><p>cemerick said: [<a href="file:aIWFiWHeGr35ImeJe5cbLA">file:aIWFiWHeGr35ImeJe5cbLA</a>]</p><p>cemerick said: [<a href="file:aI7Eu-HeGr35ImeJe5cbLA">file:aI7Eu-HeGr35ImeJe5cbLA</a>]</p><p>cemerick said: Patched attached. The <del>compat</del> one retains the current default behaviour <span class="error">&#91;*transitive-compile* true&#93;</span>, the other changes the default so that transitivity is a non-default option. At least of those I've spoken to about this, the latter is preferred.</p>
<p>The user impact of changing the default would be:</p>
<ol>
<li>The result of compiling from the REPL will change. Getting back current behaviour would require adding a <span class="error">&#91;*transitive-compile* true&#93;</span> binding to the existing bindings one must set when compiling from the REPL.</li>
<li>The same as #1 goes for those scripting AOT compilation via <tt>clojure.lang.Compile</tt> as well (whether by shell scripts, ant, etc).</li>
<li>Those using lein, clojure-maven-plugin, gradle, and others will likely have a new option provided by those tools, and perhaps a different default than the language's. I suspect those using such tools would much prefer a change from the default behaviour in any case.</li>
</ol>
<p>hlship said: Just had a brain-storm:</p>
<p>How about an option to support transitive compilation, but only if the Clojure source file being compiled as a file: URL (i.e., its a local file on the file system, not a file stored in a JAR). That would make it easier to use compilation on the local project without transitively compiling imported libraries, such as clojure-contrib.</p>
<p>So <b>transitive-compile</b> should be a keyword, not a boolean, with values :all (for 1.1 behavior), :none (to compile only the exact specified namespaces) or :local (to compile transitively, but only for local files, not source files from JARs).</p><p>cemerick said: (Crossposted to the clojure-dev list)</p>
<p>I thought about this some, and I don't think that's a good idea, at least for now. I'm uncomfortable with semantics changing depending upon where code is being loaded from &#8211; which, depending upon a tool's implementation, might be undefined. E.g. if the com.foo.bar ns is available in source form in one directory, but as classes from a jar, and classpaths aren't being constructed in a stable fashion, then the results of compilation will change.</p>
<p>If we decide that special treatment depending upon the source of code is warranted in the future, that's a fairly straightforward thing to do w.r.t. the API &#8211; we could have :all and :local as you suggest, with nil representing :none.</p><p>stu said: Rich is not comfortable enough with the implementation complexity of this patch (e.g. the guard clause for proxies and gen-class) to slide this in as a minor fix under the wire for 1.2. </p>
<p>Better to live with the pain we know a little longer than ship something we don't have enough experience with to be confident.</p><p>Updated patch to cleanly apply to HEAD and address issues raised by screening done by <a href="http://groups.google.com/group/clojure-dev/msg/0771729b72e04c9e">Cosmin Stejerean</a>. Also includes proper tests.</p>
<p><b>Note: this patch's tests require the fix for</b> <a href="http://dev.clojure.org/jira/browse/CLJ-432" title="deftype does not work if containing ns contains dashes"><del>CLJ-432</del></a>!</p><p>the "-resolved" patch resolves a conflict in main.clj</p><p>Several questions:</p>
<ol>
<li>I am getting an ant build error: "/Users/stuart/repos/clojure/build.xml:137: java.io.FileNotFoundException: Could not locate clojure/test_clojure/aot/nontransitive__init.class or clojure/test_clojure/aot/nontransitive.clj on classpath:"</li>
<li>It feels icky to have a method named writeClassFile that, under some circumstances, does <b>not</b> write a class file, but instead loads it via a dynamic loader. Maybe this is just a naming issue.</li>
<li>Are there any other ways to accomplish the goals of <tt><b>load-level</b></tt>? Or, taking the other side, if we are going to have a <b>load-level</b>, are there other possible consumers who might have different needs?</li>
<li>(Minor) Why the <tt>use :only</tt> idiom instead of just <tt>require</tt>?</li>
</ol>
<p>An alternative approach: patch write-classes-1.diff.gz</p>
<p>From <a href="https://github.com/stuartsierra/clojure/tree/write-classes">my forked branch</a></p>
<p>What this patch does:</p>
<ul>
<li>Keeps 'compile' and '<b>compile-files</b>' exactly the same</li>
<li>Adds 'compile-write-classes' to write .class files for specifically named classes</li>
<li>Minor compiler changes to support this</li>
</ul>
<p>This approach was prompted by the following observations:</p>
<ul>
<li>Java interop is the dominant reason for needing .class files</li>
<li>Things other than namespaces can generate classes for Java interop:
<ul>
<li>deftype/defrecord</li>
<li>defprotocol</li>
<li>gen-class/gen-interface</li>
</ul>
</li>
<li>For library releases, we want to control which .class files are emitted on a per-class basis, not per-namespace</li>
<li>Some legitimate uses of AOT compilation will want transitive compilation
<ul>
<li>Pre-compiling an entire application before release</li>
</ul>
</li>
</ul>
<p>S. Halloway: My apologies, I didn't know you had commented. I thought that, having assigned this issue to myself, I'd be notified of updates. <img class="emoticon" src="http://dev.clojure.org/jira/images/icons/emoticons/sad.gif" height="20" width="20" align="absmiddle" alt="" border="0"/></p>
<p>FWIW, I aim to review your comments and SS' approach over the weekend.</p><p>S. Halloway:</p>
<p>1. Certainly shouldn't happen. AFAIK, others have screened the patch, presumably with a successful build.<br/>
2. Agreed; given the approach, I think it's just a bad name.<br/>
3. Yes, I think S. Sierra's is one. See my next comment.<br/>
4. Because the <tt>:use</tt> form was already there. I've actually been using that form of <tt>:use</tt> more and more; I've found that easier than occasionally having to shuffle around specs between <tt>:use</tt> and <tt>:require</tt>. I think I'm aping Chris Houser in that regard.</p><p>I think S. Sierra's approach is fundamentally superior what I offered. I have two suggestions: one slight perspective change (which implies no change in the actual implementation), and an idea for an even simpler approach (at least from a user perspective), in that order.</p>
<p>While interop is the driving requirement behind AOT, I absolutely do not want to have to keep an updated enumeration of all of the classes resulting from each and every <tt>defrecord</tt> et al. usages in my <tt>pom.xml</tt>/<tt>project.clj</tt> (and I wouldn't wish the task of ferreting those usages and their resulting classnames on any build tool author).</p>
<p>Right now, <tt>&#42;compile-write-classes&#42;</tt> is documented to be a set of classname strings, but could just as easily be any other function. <tt>&#42;compile-write-classes&#42;</tt> should be documented to accept any predicate function (renamed to e.g. <tt>&#42;compile-write-class?&#42;</tt>?). There's no reason why it shouldn't be bound to, e.g. <tt>#(re-matches #"foo\.bar\.[\w&#95;]+$" %)</tt> if I know that all my records are defined in the <tt>foo.bar</tt> namespace.</p>
<p>To go along with that, I think some package/classname-globbing utilities along with corresponding options to <tt>clojure.lang.Compile</tt> would be most welcome. Classname munging rules are not exactly obvious, and it'd be good to make things a little easier for users in this regard.</p>
<hr />
<h5><a name="Anotheralternative"></a>Another alternative</h5>
<p>If there's a closed set of forms that generate classes that one might reasonably be interested in having in a build result (outside of use cases for pervasive AOT), then why not have a simple option that only those forms utilize? <tt>gen-class</tt> and <tt>gen-interface</tt> already do this, but reusing the all-or-nothing <tt>&#42;compile-files&#42;</tt> binding; if they keyed off of a binding that implied a diminished scope (e.g. <tt>&#42;compile-interop-forms&#42;</tt> – which would be <tt>true</tt> if <tt>&#42;compile-files&#42;</tt> were <tt>true</tt>), then they'd do exactly what we wanted. Extending this approach to <tt>deftype</tt> (and therefore <tt>defrecord</tt>) <em>should</em> be straightforward.</p>
<p>An implementation of this would probably be somewhat more complicated than S. Sierra's patch, though not as complex as my original stab at the problem (i.e. no <tt>&#42;load-level&#42;</tt>). On the plus side:</p>
<p>1. No additional configuration for users or implementation work for build tool authors, aside from the addition of the boolean diminished-scope AOT option<br/>
2. Class file generation would remain opaque from a build process standpoint<br/>
3. Future/other class-generating forms (there are a few people futzing with ASM independently, etc) can make local decisions about whether or not to participate in interop-centric classfile generation. This might be particularly helpful if a given form emits multiple classes, making the determination of a classname-based filter fn less straightforward.</p>
<p>I can see wanting to further restrict AOT to specific classnames in certain circumstances, in which case the above and S. Sierra's patch might be complimentary.</p><p>I like the idea of <tt>&#42;compile-interop-forms&#42;</tt>. But is it always possible to determine what an "interop form" is? I <em>think</em> it is, I'm just not sure.</p><p>I'm also in favor of compile-interop-forms. As far as determining, how about sticking metadata on the var?</p>
<p>(defmacro ^{:interop-form true} deftype ...)</p><p>Summary and design discussion on wiki at <a href="http://dev.clojure.org/display/design/Transitive+AOT+Compilation">http://dev.clojure.org/display/design/Transitive+AOT+Compilation</a></p><p>New attachment <tt>compile-interop-1.patch</tt> has new approach: Add a third possible value for <tt>&#42;compile-files&#42;</tt>. True and false keep their original meanings, but <tt>:interop</tt> causes <b>only</b> interop-related forms to be written out as .class files. "Interop forms" are gen-class, gen-interface, deftype, defrecord, defprotocol, and definterface.</p>
<p>Pros:</p>
<ul>
<li>doesn't change existing behavior</li>
<li>handles common case for non-transitive AOT (interop)</li>
<li>minimal changes to the compiler</li>
</ul>
<p>Cons:</p>
<ul>
<li>not flexible</li>
</ul>
<p>Just realized my patch doesn't solve the transitive compilation problem. If library A loads library B, then compiling interop forms in A will also emit interop .class files in B.</p><p>It's disappointing to see an important issue like this still unresolved after 2.5 years. This is a real pain for us. We have a large closed source project where shipping source is not an option. This forces us to manage the AOT'ing of dependencies due to the hard dependency on protocol interfaces introduced by transitive AOT compilation (see <a href="https://groups.google.com/forum/?fromgroups=#!topic/clojure-dev/r3A1JOIiwVU">https://groups.google.com/forum/?fromgroups=#!topic/clojure-dev/r3A1JOIiwVU</a>).</p><p>Paul, do you have a suggestion for which of the approaches described in comments here, or on the wiki page <a href="http://dev.clojure.org/display/design/Transitive+AOT+Compilation">http://dev.clojure.org/display/design/Transitive+AOT+Compilation</a> would be preferable solution for you? Or perhaps even a patch that implements your preferred approach?</p><p>Andy,</p>
<p>I'm now consulting with Paudi's organization, so I think I can speak for him (I'm now the default buildmeister).</p>
<p>I like Stuart's :interop idea, but that is somewhat orthogonal to our needs.</p>
<p>I return to what I would like; compilation would compile specific namespaces; dependencies of those namespaces would not be compiled.</p>
<p>To be honest, I'm still a little hung up on the interop forms: especially defprotocol and friends; from a cursory glance, it appears that todays AOT compilation will compile the protocol into a Java class, then compile the namespace that references the protocol with the assumption that the protocol's Java class is available. When we use build rules to only package our namespace's class files into the output JAR, the code fails with a NoClassDefFoundError because the protocol really needs to be recompiled, at runtime compilation, into an in-memory Java class.</p>
<p>Obviously, supporting this correctly will be a challenge; the compiled bytecode for our namespace would ideally:<br/>
1) check to see if the Java class already exists and use it if so<br/>
2) load the necessary namespaces so as to force the creation of the Java class</p>
<p>I can imagine any number of ways to juggle things to make this work, so I won't suggest a specific implementation.</p>
<p>In the meantime, our workaround is to create a "stub" module as part of our build; it simply requires in the necessary namespaces (for example, org.clojure:core.cache); this forces an AOT compile of the dependencies and we have a special rule to package such dependencies in the stub module's output JAR. This may not be a scalable problem, and it is expensive to identify what 3rd party dependencies require this treatment.</p><p>I am marking this incomplete because there does not yet seem to be plurality, much less consensus or unanimity, on approach.</p>
<p>Personally I am in favor of a solution based on a predicate that gets handed the class name and compiled bits, and then can choose whether to write the class. Pretty close to Stuart Sierra's compile-write-classes. Might be possible to flow more information than classname to the predicate.</p>
<p>Removed the 1.6 release from this and added to Release.Next list to make this a priority for the next release.</p><p>Howard,</p>
<p>I don't exactly understand your write up, I am reading the compiler, the emit-protocol macro, and emit-method-builder to try and understand it.</p>
<p>You might check to see if you have a situation similar to the following:</p>
<div class="preformatted panel" style="border-width: 1px;"><div class="preformattedContent panelContent">
<pre>(ns a.b)
(defprotocol P1
(pm [a]))
</pre>
</div></div>
<p>then either</p>
<div class="preformatted panel" style="border-width: 1px;"><div class="preformattedContent panelContent">
<pre>(ns a.c
(:import (a.b P1))
(defrecord R []
P1
(pm [x] x))
</pre>
</div></div>
<p>or</p>
<div class="preformatted panel" style="border-width: 1px;"><div class="preformattedContent panelContent">
<pre>(ns a.c)
(defrecord R []
a.b.P1
(pm [x] x))
</pre>
</div></div>
<p>in both examples defrecord is actually getting the class behind the protocol instead of the protocol, the correct thing to do is</p>
<div class="preformatted panel" style="border-width: 1px;"><div class="preformattedContent panelContent">
<pre>(ns a.c
(:require [a.b :refer [P1]]))
(defrecord R []
P1
(pm [x] x))
</pre>
</div></div>
<p>This is an <b>extremely</b> common mistake people make when using protocols, unfortunately the flexibility of using interfaces directly in defrecord forms, and protocols being backed by interfaces means it is very easy to unwittingly make such a mistake. Both of the mistake examples could result in missing classes/namespace problems.</p><p>The write-classes patch still applies cleanly, I just tried it out, made a one-line change to lein-ring and got a war-file for my app with two generated classes instead of hundreds. This seems like a good fix to me. What do we need to do to get it into release?</p><p>As Stu said above, we do not have consensus on whether this is the right way to go with this. I am planning to look at all outstanding AOT tickets (including this one) early in 1.8.</p>ApprovalVettedGlobal RankPatchCode and TestWaiting Oncemerick[CLJ-1148] adds docstring support to defoncehttp://dev.clojure.org/jira/browse/CLJ-1148
Clojure<p>Pass all args from <tt>defonce</tt> on to <tt>def</tt> so it supports docstrings (or potentially other future features) just like def.</p>
<p>Docstrings and other Var metadata will be lost when the <tt>defonce</tt> is reëvaluated.</p>
<p><b>Patch:</b> clj-1148-defonce-6.patch</p>
<p><b>Screened by:</b> </p>CLJ-1148adds docstring support to defonceEnhancementMinorOpenUnresolvedUnassignedJoe GallodocstringThu, 17 Jan 2013 17:16:40 -0600Wed, 29 Apr 2015 10:44:18 -0500Release 1.574<p>Changed to defect for stomping metadata.</p><p>Please add tests. The clojure.test-helper namespace has useful temporary namespace support.</p><p>This new patch includes the changes to defonce and also tests.</p><p>Changing to Vetted so this is screenable again.</p><p>I disagree about the stomp metadata - different metadata was provided. The purpose of defonce is to avoid the re-evaluation of the init. Is this the simplest change that accomplishes the doc string? In any case split in two.</p><p>Reduced scope of ticket to just passing defonce args on to def to add support for docstring. Added new patch that does this.</p><p>Screened clj-1148-defonce-2.patch but returning to 'incomplete' status.</p>
<p>The <tt>:arglists</tt> metadata in this patch (a list of symbols) is inconsistent with all other uses of <tt>:arglists</tt> (a list of vectors).</p>
<p>Other than that the patch is good.</p><p>Updated patch to address inconsistency in arglist format and attached clj-1148-defonce-3.patch.</p><p>The patch clj-1148-defonce-3.patch is OK but it doesn't really address the docstring issue because <tt>defonce</tt> still destroys metadata. For example:</p>
<div class="code panel" style="border-width: 1px;"><div class="codeContent panelContent">
<pre class="code-java">user=&gt; (defonce foo <span class="code-quote">"docstring <span class="code-keyword">for</span> foo"</span> (<span class="code-keyword">do</span> (prn 42) 42))
42
#'user/foo
user=&gt; (doc foo)
-------------------------
user/foo
docstring <span class="code-keyword">for</span> foo
nil
user=&gt; (defonce foo <span class="code-quote">"docstring <span class="code-keyword">for</span> foo"</span> (<span class="code-keyword">do</span> (prn 42) 42))
nil
user=&gt; (doc foo)
-------------------------
user/foo
nil</pre>
</div></div><p>Screened with reservations noted.</p><p>Stuart is right, second defonce should retain the doc string (since it again provides it, should be no-op)</p><p>pull out of 1.6</p><p>This version looks for previously defined var with <tt>resolve</tt>. A repeated defonce won't affect the namespace at all if the variable is already defined and bounded.</p>
<p>Please confirm using <tt>(resolve '~name)</tt> is not a problem w.r.t <tt>ns</tt>-bindings or similar.</p>
<p>This patch also contains the tests from <tt>clj-1148-defonce-3.patch</tt> as well as the <tt>:arglists</tt> property.</p>
<p>(patch 4 missed one def-row, sorry for mailbox noise).</p><p>Yet another, simpler version of defonce. No test-cases included. </p>
<p>This version just makes an <tt>(or (nil? v#) (not (.hasRoot v#))</tt> test on the resolved variable. If this is true, really define by <tt>(def ~name ~@args)</tt> else do nothing.</p><p>Linus, while JIRA can handle multiple attachments on the same ticket with identical names, it can be confusing to do so. Would you mind renaming or removing the ones you have added with duplicate names, e.g. delete obsolete ones, perhaps? Instructions for deleting patches are in the "Removing patches" section on this wiki page: <a href="http://dev.clojure.org/display/community/Developing+Patches">http://dev.clojure.org/display/community/Developing+Patches</a></p>ApprovalVettedGlobal RankPatchCode and Test[CLJ-1099] better error message when passing non-seq to seqhttp://dev.clojure.org/jira/browse/CLJ-1099
Clojure<p>Design discussion <a href="http://dev.clojure.org/display/design/Better+Error+Messages">here</a>.</p>
<p>This patch improves Clojure's error message for a single common error: passing a non-seq where a seq is neede. More importantly, it is intended as a prototype for other similar improvements in the future.</p>
<p>Error message before:</p>
<div class="preformatted panel" style="border-width: 1px;"><div class="preformattedContent panelContent">
<pre>(cons 1 2)
=&gt; IllegalArgumentException Don't know how to create ISeq from: java.lang.Long
</pre>
</div></div>
<p>Error message after:</p>
<div class="preformatted panel" style="border-width: 1px;"><div class="preformattedContent panelContent">
<pre>user=&gt; (cons 1 2)
ExceptionInfo Don't know how to create ISeq from: java.lang.Long
user=&gt; (ex-data *e)
{:instance 2}
</pre>
</div></div>
<p><b>Patch:</b> better-error-message-for-seq.patch<br/>
NOTE: This patch was reverted as it affected the inlining of RT.seqFrom().</p>CLJ-1099better error message when passing non-seq to seqEnhancementMinorReopenedUnresolvedUnassignedStuart HallowaycheckargsThu, 1 Nov 2012 10:03:14 -0500Tue, 24 Feb 2015 11:57:23 -060013<p>Wouldn't it be better to make it read "Don't know how to create ISeq from: 2 (java.lang.Long)"? How many beginners will figure<br/>
out ex-data exists and how to use it?</p><p>Hi Michael,</p>
<p>ex-info messages should not, in general, pr-str things into their bodies. This raises the question of print-length and print-level in a place where the user doesn't have good control, while the whole point of ex-info is to be in the data business, not the string business. Users can control printing from ex-data any way they like.</p>
<p>There are two possible ways to make beginners aware of ex-data: Tell them about it in one (or a few places) in docs, or in an infinite number of places saying "This would have been useful here, but we didn't use it because you might not know about it." I prefer the former.</p>
<p>That said, I think it would be great to increase the visibility of ex-info and ex-data early on in documentation for beginners, and to make sure that things like exception printing in logs are flexible enough not to lose the benefits of ex-info.</p><p>Just a comment that this fix was committed before release 1.6.0, and then reverted very shortly before release 1.6.0. I believe the reason for reverting was due to concerns that this change made performance about 5% slower in some relatively common cases, with a suspicion that it could have affected inlining of the seqFrom method.</p>
<p>Not sure whether the ticket should be reopened or not.</p>ApprovalVettedGlobal RankPatchCode[CLJ-865] Macroexpansion discards &form metadatahttp://dev.clojure.org/jira/browse/CLJ-865
Clojure<p>This patch changes the behavior of metadata when used in conjunction with macros. The metadata &amp;form is now merged with the metadata of the macro call sexpr. This allows users to either type-hint the inner or the outer form in a macro call and have somewhat better results. In the past, the metadata from the macroexpand was used as-is. This disallowed code like the following, to work without reflection:</p>
<p>(.trim ^String (when true "hello "))</p>
<p><b>Patch:</b> 2013-10-11_<a href="http://dev.clojure.org/jira/browse/CLJ-865" title="Macroexpansion discards &amp;form metadata">CLJ-865</a>_Fix-With-Tests.diff<br/>
<b>Screened by:</b> Timothy Baldridge</p>
<p>--------- Implementation Details ----------</p>
<p>As discussed in <a href="http://groups.google.com/group/clojure/browse_thread/thread/2690cb6ca0e8beb8">http://groups.google.com/group/clojure/browse_thread/thread/2690cb6ca0e8beb8</a> there is a "surprise factor" when type-hinting an expression that represents a macro, such as with (.length ^String (doto (identity "x") prn)). Here the doto macro discards the metadata on &amp;form, causing a reflective lookup. This has the effect that while expressions representing function calls can be type-hinted, expressions representing macros in general cannot. The doto macro could be rewritten to respect its &amp;form metadata, but doing this for every macro in existence would be tedious and error-prone. Instead, I propose a change to the compiler, to cause macroexpansion to hang onto the metadata automatically.</p>
<p>The first patch attached adds a test for the behavior I propose: this test fails. After applying the second patch, the test passes.</p>
<p>There are a couple points that merit further consideration before accepting my patch:</p>
<ul class="alternate" type="square">
<li>I'm not sure I actually got the Java code formatted correctly. My editor is not well-configured to get the clojure/core style right automatically.</li>
<li>My solution is to take the &amp;form metadata, drop :line/:file keys, and then merge with the returned metadata, with &amp;form taking precedence. I'm not sure whether this is the right approach in all cases, even though it works for :tag metadata.</li>
<li>I achieved this with a change to the compiler, which makes it fairly heavy-weight. It should be possible to instead adjust defmacro if changes to the compiler are not desirable. However, I believe this would involve substantially more work and be harder to test (for example, multiple arities complicate things). It seems nicer to treat the macroexpansion as a black box and then make metadata tweaks to the result, rather than modifying their actual defmacro code.</li>
<li>If a macro expands to something that is not an IObj, such as an Integer, then my patch silently discards the caller's metadata. Would it be better to throw an exception?</li>
</ul>
CLJ-865Macroexpansion discards &form metadataEnhancementMinorOpenUnresolvedUnassignedAlan MalloyCompilerWed, 26 Oct 2011 15:19:11 -0500Fri, 5 Dec 2014 13:54:00 -06002010<p>So I went ahead and did the work of making this change in clojure.core/defmacro instead of clojure.lang.Compiler/macroexpand1. It was even worse than I expected: I didn't realize we don't yet have syntax-quote or apply at this stage in bootstrapping, so writing a non-trivial macroexpansion requires a huge amount of (list `foo (list `bar 'local-name)) and so forth.</p>
<p>I'm sure the version I wrote is not optimal, but it seemed simpler to piggyback on defn, and then use alter-var-root to shim the metadata management in, than it would have been to expand to the correct thing in the first place.</p>
<p>Anyway, attached patch #3 could be applied instead of #2 to resolve the issue in clojure.core instead of clojure.lang. The tests added in patch #1 pass either way.</p><p>I realized I can do this with a named private function instead of an anonymous function, reducing the amount of mess defmacro itself has to generate. Patch 4 is, I think, strictly better than Patch 3, if a Clojure implementation is preferred to one in Java.</p><p>I prefer patch 0002 in Java over either 0003 or 0004. Patch 0002 keeps the knowledge of how to invoke macro fns (specifically the extra &amp;form and &amp;env args) in one place, macroexpand1 rather than duplicating that knowledge in core.clj as well. Note patch 0001 is just tests.</p>
<p>The proposed default macroexpansion behavior is more useful than what we currently have, but there are two details I'd like to think about a bit more:</p>
<p>1) In exchange for a more useful default, macro writers lose the ability to consume their &amp;form metadata and have control over the resulting form metadata without the &amp;form metadata overridding it. That is, macros are no longer in complete control of their output form.</p>
<p>2) Rule (1) above has hardcoded exceptions for :line and :file, where &amp;form metadata is unable to override the results returned by the macro.</p><p>This patch incorporates all previous patches to this issue.</p>
<p>On the clj-dev mailing list, Andy Fingerhut suggested a new metadata key for allowing the macro author to specify "I've looked at their &amp;form metadata, and this form is exactly what I want to expand to, please don't change the metadata any further." I've implemented this, and I think it addresses Chouser's concern about needing a way to "break out" of the improved-default behavior.</p>
<p>One open question is, is :explicit-meta the right key to use? I spent some time tracking down a bug caused by my forgetting the keyword and using :explicit-metadata in my test; perhaps something more difficult to get confused by is available.</p><p>clj-865-updated-v2-patch.txt dated Aug 14 2013 is identical to Alan Malloy's updated.patch dated Jun 1 2012. I simply updated the patch to apply cleanly to latest master after some context lines in the test file macros.clj had gone bad due to recent commits.</p><p>Added updated patch that works against master, and also removes COLUMN_KEY from the macro's metadata</p><p>Added patch that contains all fixes plus a few more tests. </p><p>Since this could break things, we could just take metadata on the macro name to ask for this:</p>
<div class="code panel" style="border-width: 1px;"><div class="codeContent panelContent">
<pre class="code-java">(defmacro ^:keep-meta simple-macro [f arg]
`(~f ~arg))</pre>
</div></div>
<p>or something</p><p>Sure, I'll put together that patch. I'm worried, though, that if it's not the default, it will just never get used, and we'll be in effectively the same situation we are now, where no macros do this right. I don't foresee anyone going through their libraries to add ^:keep-meta on every macro.</p><p>I updated the patch to behave as Rich requested, but it caused a test regression that I can't figure out, in the handling of either refer or private vars. Hopefully someone else can run the tests and figure out what is missing here; my change is supposed to be opt-in, and I can't see where I've gone wrong.</p><p>Alan, your patch clj865.patch dated Dec 3, 2013 has some HTML cruft at the beginning and end, but even after removing that it does not apply cleanly to the latest Clojure master as of today. I understand that you say it needs more work, but it would be easier for others who wish to try it out if it applied cleanly.</p><p>Sorry Andy, and thanks for noticing. I haven't been on a very developer-friendly computer recently, but I'll try to fix the patch tonight.</p><p>Here's a fix to the patch. I verified that this applies cleanly to current master.</p><p>To clarify, it's the file named clj-865.patch. I didn't realize JIRA wouldn't make it clear which file I uploaded along with the comment.</p><p>As of Eastwood version 0.2.0, it includes a new warning :unused-meta-on-macro that will warn whenever metadata is applied to a macro invocation, with the known exception of clojure.core/fn, which explicitly uses metadata applied to it. <a href="https://github.com/jonase/eastwood#unused-meta-on-macro">https://github.com/jonase/eastwood#unused-meta-on-macro</a></p>ApprovalVettedGlobal RankPatchCode and Test[CLJ-124] GC Issue 120: Determine mechanism for controlling automatic shutdown of Agents, with a default policy and mechanism for changing that policy as neededhttp://dev.clojure.org/jira/browse/CLJ-124
Clojure<p>The original description when this ticket was vetted is below, starting with "Reported by cemer...@snowtide.com, June 01, 2009". This prefix attempts to summarize the issue and discussion.</p>
<p><b>Description</b>:</p>
<p>Several Clojure functions involving agents and futures, such as future, pmap, clojure.java.shell/sh, and a few others, create non-daemon threads in the JVM in an ExecutorService called soloExecutor created via Executors#newCachedThreadPool. The javadocs for this method here <a href="http://docs.oracle.com/javase/7/docs/api/java/util/concurrent/Executors.html#newCachedThreadPool%28%29">http://docs.oracle.com/javase/7/docs/api/java/util/concurrent/Executors.html#newCachedThreadPool%28%29</a> say "Threads that have not been used for sixty seconds are terminated and removed from the cache." This causes a 60-second wait after a Clojure program is done before the JVM process exits. Questions about this confusing behavior come up a couple of times per year on the Clojure Google group. Search for "shutdown-agents" to find most of these occurrences, since calling (shutdown-agents) at the end of one's program typically eliminates this 60-second wait.</p>
<p><b>Example</b>:</p>
<div class="code panel" style="border-width: 1px;"><div class="codeContent panelContent">
<pre class="code-java">% java -cp clojure.jar clojure.main -e <span class="code-quote">"(println 1)"</span>
1
[ <span class="code-keyword">this</span> <span class="code-keyword">case</span> exits quickly ]
% java -cp clojure.jar clojure.main -e <span class="code-quote">"(println @(<span class="code-keyword">future</span> 1))"</span>
1
[ 60-second pause before process exits, at least on many platforms and JVMs ]</pre>
</div></div>
<p><b>Summary of comments before July 2014</b>:</p>
<p>Most of the comments on this ticket on or before August 23, 2010 were likely spread out in time before being imported from the older ticket tracking system into JIRA. Most of them refer to an older suggested patch that is not in JIRA, and compilation problems it had with JDK 1.5, which is no longer supported by Clojure 1.6.0. I think these comments can be safely ignored now.</p>
<p>Alex Miller blogged about this and related issues here: <a href="http://tech.puredanger.com/2010/06/08/clojure-agent-thread-pools/">http://tech.puredanger.com/2010/06/08/clojure-agent-thread-pools/</a></p>
<p>Since then, two of the suggestions Alex raised have been addressed. One by <a href="http://dev.clojure.org/jira/browse/CLJ-378" title="Set thread names on agent thread pools"><del>CLJ-378</del></a> and one by the addition of set-agent-send-executor! and similar functions to Clojure 1.5.0: <a href="https://github.com/clojure/clojure/blob/master/changes.md#23-clojurecoreset-agent-send-executor-set-agent-send-off-executor-and-send-via">https://github.com/clojure/clojure/blob/master/changes.md#23-clojurecoreset-agent-send-executor-set-agent-send-off-executor-and-send-via</a></p>
<p>One remaining issue is the topic of this ticket, which is how best to avoid this 60-second pause.</p>
<p><b>Approach #1: automatically shut down agents</b></p>
<p>One method is mentioned in Chas Emerick's original description below, suggested by Rich Hickey, but perhaps long enough ago he may no longer endorse it: Create a Var &#42;auto-shutdown-agents&#42; that when true (the default value), clojure.lang.Agent shutdown() is called after the clojure.main entry point. This removes the surprising wait for common methods of starting Clojure, while allowing expert users to change that value to false if desired.</p>
<p><b>Approach #2: create daemon threads by default</b></p>
<p>Another method mentioned by several people in the comments is to change the threads created in agent thread pools to daemon threads by default, and perhaps to deprecate shutdown-agents or modify it to be less dangerous. That approach is discussed a bit more in Alex's blog post linked above, and in a comment from Alexander Taggart on July 11, 2011 below.</p>
<p><b>Approach #3:</b></p>
<p>The only other comment before 2014 that is not elaborated in this summary is shoover's suggestion: 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.</p>
<p><b>Approach #4: Create a cached thread pool with a timeout much lower than 60 seconds</b></p>
<p>This could be done by using one of the ThreadPoolExecutor constructors with a keepAliveTime parameter of the desired time.</p>
<p><b>Patch</b>: clj-124-v1.patch clj-124-daemonthreads-v1.patch</p>
<p>At most one of these patches should be considered, depending upon the desired approach to take.</p>
<p>Patch clj-124-v1.patch implements appproach #1 using &#42;auto-shutdown-agents&#42;. See the Jul 31 2014 comment when this patch was added for some additional details.</p>
<p>Patch clj-124-daemonthreads-v1.patch implements approach #2 and is straightforward.</p>
<div class="code panel" style="border-width: 1px;"><div class="codeContent panelContent">
<pre class="code-java">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:<span class="code-comment">//groups.google.com/group/clojure/browse_thread/thread/409054e3542adc1f)
</span>and in #clojure about some clojure scripts hanging, either <span class="code-keyword">for</span> 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 <span class="code-keyword">for</span> <span class="code-keyword">this</span> 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 <span class="code-keyword">finally</span> block in
clojure.lang.Compile/main resolved the problem; a patch including <span class="code-keyword">this</span>
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 <span class="code-keyword">this</span>
problem in different directions. It's a very difficult thing to debug <span class="code-keyword">if</span>
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* <span class="code-keyword">var</span>, which:
- <span class="code-keyword">if</span> <span class="code-keyword">true</span> when exiting one of the main entry points (clojure.main, or the
legacy script/repl entry points), Agent.shutdown() would be called,
allowing <span class="code-keyword">for</span> the clean exit of the application
- would be bound by <span class="code-keyword">default</span> to <span class="code-keyword">true</span>
- could be easily set to <span class="code-keyword">false</span> <span class="code-keyword">for</span> anyone with an advanced use-<span class="code-keyword">case</span> 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 <span class="code-keyword">this</span> may represent the best compromise between
least-surprise and maximal functionality <span class="code-keyword">for</span> 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 <span class="code-keyword">default</span> thread pool executor, which
results in a 60s keepalive. Lowering <span class="code-keyword">this</span> 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 <span class="code-keyword">this</span>
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 <span class="code-keyword">for</span> either of the suggestions mentioned
above...</pre>
</div></div>CLJ-124GC Issue 120: Determine mechanism for controlling automatic shutdown of Agents, with a default policy and mechanism for changing that policy as neededEnhancementMinorOpenUnresolvedAlex MillerChas EmerickagentsWed, 17 Jun 2009 23:24:00 -0500Sat, 23 Aug 2014 11:52:33 -0500Release 1.5Release 1.686<p>Converted from <a href="http://www.assembla.com/spaces/clojure/tickets/124">http://www.assembla.com/spaces/clojure/tickets/124</a><br/>
Attachments:<br/>
compile-agent-shutdown.patch - <a href="https://www.assembla.com/spaces/clojure/documents/a56S2ow4ur3O2PeJe5afGb/download/a56S2ow4ur3O2PeJe5afGb">https://www.assembla.com/spaces/clojure/documents/a56S2ow4ur3O2PeJe5afGb/download/a56S2ow4ur3O2PeJe5afGb</a><br/>
124-compilation.diff - <a href="https://www.assembla.com/spaces/clojure/documents/aqn0IGxZSr3RUGeJe5aVNr/download/aqn0IGxZSr3RUGeJe5aVNr">https://www.assembla.com/spaces/clojure/documents/aqn0IGxZSr3RUGeJe5aVNr/download/aqn0IGxZSr3RUGeJe5aVNr</a></p><p>oranenj said: [<a href="file:a56S2ow4ur3O2PeJe5afGb">file:a56S2ow4ur3O2PeJe5afGb</a>]</p><p>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)</p><p>cemerick said: (In [<span class="error">&#91;r:fa3d24973fc415b35ae6ec8d84b61ace76bd4133&#93;</span>]) Add a call to Agent.shutdown() at the end of clojure.lang.Compile/main Refs #124</p>
<p>Signed-off-by: Chouser &lt;chouser@n01se.net&gt;</p>
<p>Branch: master</p><p>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 <b>auto-shutdown-agents</b> 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.</p><p>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.</p>
<p>Java 1.5.0_19<br/>
Java 1.6.0_13</p>
<p>For example, when building clojure using "ant" from within my clone of the clojure repo:</p>
<p><span class="error">&#91;java&#93;</span> java.security.AccessControlException: access denied (java.lang.RuntimePermission modifyThread)<br/>
<span class="error">&#91;java&#93;</span> at java.security.AccessControlContext.checkPermission(AccessControlContext.java:264)<br/>
<span class="error">&#91;java&#93;</span> at java.security.AccessController.checkPermission(AccessController.java:427)<br/>
<span class="error">&#91;java&#93;</span> at java.util.concurrent.ThreadPoolExecutor.shutdown(ThreadPoolExecutor.java:894)<br/>
<span class="error">&#91;java&#93;</span> at clojure.lang.Agent.shutdown(Agent.java:34)<br/>
<span class="error">&#91;java&#93;</span> at clojure.lang.Compile.main(Compile.java:71)</p>
<p>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.</p><p>chouser@n01se.net said: I had only tested it on my ubuntu box &#8211; looks like that was openjdk 1.6.0_0. I'll test again with sun-java5 and sun-java6.</p><p>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?</p><p>achimpassen said: [<a href="file:aqn0IGxZSr3RUGeJe5aVNr">file:aqn0IGxZSr3RUGeJe5aVNr</a>]</p><p>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.</p><p>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.</p><p>chouser@n01se.net said: (In [<span class="error">&#91;r:895b39dabc17b3fd766fdbac3b0757edb0d4b60d&#93;</span>]) Rev fa3d2497 causes compile to fail on some VMs &#8211; back it out. Refs #124</p>
<p>Branch: master</p><p>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.</p>
<p>See the note for "permissions" on <a href="http://ant.apache.org/manual/CoreTasks/java.html">http://ant.apache.org/manual/CoreTasks/java.html</a> . I assume ThreadPoolExecutor.shutdown is the problem, it would shutdown the main Ant thread, so Ant disallows that. Forking avoids the permissions limitation.</p>
<p>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.</p><p>chouser@n01se.net said: I don't know if the &lt;java fork=true&gt; patch is a good idea or not, or if there's a better way to solve the original problem.</p>
<p>Chas, I'm kicking back to you, but I guess if you don't want it you can reassign to "nobody".</p><p>richhickey said: Updating tickets (#8, #42, #113, #2, #20, #94, #96, #104, #119, #124, #127, #149, #162)</p><p>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.</p>
<p>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.</p><p>alexdmiller said: I blogged about these issues at:<br/>
<a href="http://tech.puredanger.com/2010/06/08/clojure-agent-thread-pools/">http://tech.puredanger.com/2010/06/08/clojure-agent-thread-pools/</a></p>
<p>I think that:</p>
<ul>
<li>agent thread pool threads should be named (see ticket <a href="https://www.assembla.com/spaces/clojure/tickets/378-set-thread-names-on-agent-thread-pools">#378</a>)</li>
<li>agent thread pools must be daemon threads by default</li>
<li>having ways to specify an customized executor pool for an agent send/send-off is essential to customize threading behavior</li>
<li>(shutdown-agents) should be either deprecated or made less dangerous</li>
</ul>
<p>Rich, what is the intention behind using non-daemon threads in the agent pools?</p>
<p>If it is because daemon threads could terminate before their work is complete, would it be acceptable to <a href="http://download.oracle.com/javase/1.5.0/docs/api/java/lang/Runtime.html#addShutdownHook(java.lang.Thread)">add a shutdown hook</a> to ensure against such premature termination? Such a shutdown hook could call <tt>Agent.shutdown()</tt>, then <a href="http://download.oracle.com/javase/1.5.0/docs/api/java/util/concurrent/ExecutorService.html#awaitTermination(long, java.util.concurrent.TimeUnit)"><tt>awaitTermination()</tt></a> on the pools.</p><p>Moving this ticket out of approval "OK" status, and dropping the priority. These were Assembla import defaults.</p>
<p>Also, Chas gets to be the Reporter now.</p><p>Heh, blast from the past.</p>
<p>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 <a href="https://github.com/clojure/clojure/commit/f5f4faf">recent enhancements</a> that address some of the pain points that Alex Miller mentioned above.</p>
<p>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).</p><p>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.</p>
<p>People send messages to the Clojure Google group every couple of months hitting this issue, and one even filed <a href="http://dev.clojure.org/jira/browse/CLJ-959" title="after call to clojure.java.shell/sh, jvm won&#39;t exit"><del>CLJ-959</del></a> 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.</p><p>Patch clj-124-v1.patch dated Jul 31 2014 implements the approach of calling clojure.lang.Agent#shutdown when the new Var &#42;auto-shutdown-agents&#42; is true, which is its default value.</p>
<p>I don't see any benefit to making this Var dynamic. Unless I am missing something, only the root binding value is visible after clojure.main/main returns, not any binding that would be pushed on top of that if it were dynamic. It seems to require alter-var-root to change it to false in a way that this patch would avoid calling clojure.lang.Agent#shutdown.</p>
<p>This patch only adds the shutdown call to clojure.main#main, but can easily be added to the legacy_repl and legacy_script methods if desired.</p><p>Patch clj-124-daemonthreads-v1.patch dated Aug 23 2014 simply modifies the ThreadFactory so that every thread created in an agent thread pool is a daemon thread.</p>ApprovalVettedGlobal RankPatchCodeWaiting Onrichhickey