<< Back to previous view

[UNIFY-2] Remove reflection warnings Created: 05/Jan/12  Updated: 05/Jan/12  Resolved: 05/Jan/12

Status: Resolved
Project: core.unify
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Fogus Assignee: Fogus
Resolution: Completed Votes: 0
Labels: performance, reflection, unify

Approval: Ok

 Description   
Reflection warning, clojure/core/unify.clj:30 - reference to field getClass can't be resolved.
Reflection warning, clojure/core/unify.clj:30 - reference to field isArray can't be resolved.

These reflective calls occur frequently enough that they should be resolved.



 Comments   
Comment by Fogus [ 05/Jan/12 7:37 AM ]

Fixed in 6b6d1130bf857439d1863f6fc574a7a6541b84b8.





[NREPL-42] Reflection warnings in users' projects Created: 10/Jun/13  Updated: 19/Aug/14  Resolved: 06/Aug/13

Status: Closed
Project: tools.nrepl
Component/s: None
Affects Version/s: 0.2.2
Fix Version/s: 0.2.4

Type: Defect Priority: Minor
Reporter: John Hume Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: reflection

Attachments: Text File 0001-Eliminate-some-reflective-calls.patch    
Patch: Code

 Description   

In a project that uses clojure.tools.nrepl.server, we see a handful of reflection warnings from nrepl.

The attached patch eliminates those.



 Comments   
Comment by John Hume [ 11/Jun/13 9:14 AM ]

I should clarify that this is an issue in 0.2.3, but JIRA doesn't know about that version yet.

Comment by Chas Emerick [ 06/Aug/13 6:02 AM ]

Patch applied @ 1c53a172a8, thanks!

There is one remaining reflection warning, only because I culled the "fix" from your patch; the reflection in that case is helping us avoid the extra code IMO.





[CLJS-387] Add docstring from def and ns definitions to @namespaces metadata map, and make reflect functions make use of that Created: 07/Oct/12  Updated: 27/Jul/13  Resolved: 17/Oct/12

Status: Closed
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Frank Siebenlist Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: docs, enhancement, patch,, reflection
Environment:

clojure/clojurescript "0.0-1450"


Attachments: File add-ns-def-doc-patch.diff    
Patch: Code

 Description   

The docstrings were parsed from the definitions-forms for def and ns, but not added to the @namespaces metadata map.
There is no :doc entry used in the ns' metadata in the @namespaces.
No ns's :doc info is communicated to the browser in the reflect functions with reflect/doc.
Patch-file is attached with code-changes that add the :doc info for ns and def to the @namespaces, and enhances the reflect functions to communicate that info to the browser in the reflect/doc call.



 Comments   
Comment by David Nolen [ 15/Oct/12 11:06 PM ]

This patch no longer applies, mind updating it?

Comment by Frank Siebenlist [ 16/Oct/12 12:10 AM ]

This patch should apply to master version on Mon, 15 Oct 2012 22:03:19 -0700 (4defcbcf19112b9be6a4a27b5d8855552bf94948)

Comment by David Nolen [ 17/Oct/12 10:57 AM ]

Excellent, fixed http://github.com/clojure/clojurescript/commit/bef56a74f2eeecabfe0c0a28d89b455dce576ea3

Please at the ticket # to the commit message though, thanks!





[CLJ-1929] Can't typehint literal collection to avoid reflection on Java interop call Created: 16/May/16  Updated: 18/May/16

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

Type: Defect Priority: Major
Reporter: David B├╝rgin Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: interop, reflection, typehints
Environment:

OS X 10.11.4


Attachments: Text File 0001-CLJ-1929-preserve-type-hints-in-literals.patch    
Patch: Code
Approval: Triaged

 Description   

There is a reflection warning when passing a Clojure collection to a method that has a parameter of a collections interface type like java.util.Map.

Example calling java.time.format.DateTimeFormatterBuilder.appendText(java.time.temporal.TemporalField, java.util.Map):

(import 'java.time.format.DateTimeFormatterBuilder
        'java.time.format.TextStyle
        'java.time.temporal.ChronoField)

(set! *warn-on-reflection* true)

(let [builder (DateTimeFormatterBuilder.)]
  (.appendText builder ChronoField/YEAR {}))
; Reflection warning, NO_SOURCE_PATH:6:3 - call to method appendText on java.time.format.DateTimeFormatterBuilder can't be resolved (argument types: java.time.temporal.ChronoField, clojure.lang.IPersistentMap).

The map literal cannot be hinted:

(let [builder (DateTimeFormatterBuilder.)]
  (.appendText builder ChronoField/YEAR ^java.util.Map {}))
; Reflection warning, NO_SOURCE_PATH:8:3 - call to method appendText on java.time.format.DateTimeFormatterBuilder can't be resolved (argument types: java.time.temporal.ChronoField, clojure.lang.IPersistentMap).

The warning does not appear when the map is not empty:

(let [builder (DateTimeFormatterBuilder.)]
  (.appendText builder ChronoField/YEAR {1 "a"}))

Nor does it appear on similar methods where there is no overloaded method with the same arity:

(let [builder (DateTimeFormatterBuilder.)]
  (.appendZoneText builder TextStyle/FULL #{}))

Workaround is to not use a literal:

(let [builder (DateTimeFormatterBuilder.)]
  (.appendText builder ChronoField/YEAR ^java.util.Map (array-map)))

It should be possible to infer in these cases like elsewhere that {} implements java.util.Map.

If that is not viable a type hint on {} should be honored.

Approach: preserve user hints in literal collections
Patch: 0001-CLJ-1929-preserve-type-hints-in-literals.patch






[CLJ-1921] Wrong numeric result from Math/abs on Java 8 Created: 09/May/16  Updated: 10/May/16

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

Type: Defect Priority: Major
Reporter: Alex Miller Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: math, reflection
Environment:

does not seem specific to Clojure version
occurs only in Java 1.8



 Description   

This is with Java 1.8 (Oracle or Open JDK):

weird-abs.core=> (Math/abs -10000000000000)
10000000000000
weird-abs.core=> (def a    -10000000000000)
#'weird-abs.core/a
weird-abs.core=> (Math/abs a)
1316134912

In Java 1.7, the expected results are returned instead (10000000000000).

Cause: It appears that Math.abs(int) is being invoked. Both the int and long versions are considered by the reflector but Java 1.7 and 1.8 return these signatures in different orders and the first one found is picked.

Workaround: Use hint or cast to inform the reflector which one to pick.



 Comments   
Comment by Alex Miller [ 09/May/16 9:03 AM ]

In the first case, -10000000000000 is a long and the compiler unambiguously finds Math.abs(long).

In the second case, a is an Object and all abs signatures are considered (this is in Reflector.invokeMatchingMethod). In both Java 1.7 and 1.8, the long and int signatures are found "congruent".

In Java 1.7, the long version is found first and treated as a match, then int is checked and Compiler.subsumes([int], [long]) returns false, leading to the long method being kept as the match.

In Java 1.8, the int version is found first and treated as a match, then long is checked and Compiler.subsumes([long], [int]) returns false, leading to the int method being kept as the match.

Both of these return false on both JDKs:

(Compiler/subsumes (into-array [Long/TYPE]) (into-array [Integer/TYPE]))
(Compiler/subsumes (into-array [Integer/TYPE]) (into-array [Long/TYPE]))

so the real difference is just the ordering that is considered, which is JDK-specific.

The considered signatures could be sorted in some canonical way making this behavior consistent, or could maybe express a preference between the two signatures somehow.

In any case, getting rid of the reflection here by hinting or casting a resolves the problem - it should be considered only luck not intention that the correct results comes out with Java 7 in this case, I think.

Comment by Nicola Mometto [ 10/May/16 7:58 AM ]

It seems to me that the non-deterministic behaviour of clojure's reflector of randomly picking one overload when more than one is available is both highly counterintuitive and undesirable.

IMHO the only sane approach would be to:

  • pick the most specific type if possible (e.g. if what's available is [Object, CharSequence, String] and the reflected type is a StringBuffer, we use CharSequence rather than Object.
  • pick the widest primitive type if possible (e.g. in this case we'd use long rather than int)
  • Fail with a `More than one matching method found` exception if conflicts can't be resolved (this already happens in some cases)

(I'm still scarred from previous experiences of reading/patching the complex beast that is Reflector.java and the reflective bits of Compiler.java, so I propose this with no intention of ever working on this myself )

Comment by Alex Miller [ 10/May/16 8:03 AM ]

I think the subsumes check is effectively trying to do your option #1 already - this is a case where the types of the arguments in the two cases have no hierarchical relationship. Probably #2 would make more sense - expressing a preference, although there are certainly cases where "widest" has no meaning, so not sure what the general form of this is.

Comment by Nicola Mometto [ 10/May/16 8:05 AM ]

To clarify, that wasn't a list of different options, it was a list of steps to take.
i.e. if it's possible to pick the most specific type from a hierarchy, do that, ELSE if the types are primitive, pick the widest ELSE fail





[CLJ-1866] Optimise argument boxing during reflection Created: 08/Dec/15  Updated: 11/Dec/15

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

Type: Enhancement Priority: Minor
Reporter: Mike Anderson Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: performance, reflection

Attachments: Text File clj-1866.patch    

 Description   

Currently argument boxing is clojure.lang.Reflector is inefficient for the following two reasons:
1. It makes an unnecessary call to Class.cast(..) when the parameter type is non-primitive
2. It allocates an unnecessary extra Object[] array when boxing arguments

This patch fixes these issues, without otherwise changing behaviour. All tests pass.



 Comments   
Comment by Alex Miller [ 09/Dec/15 7:23 AM ]

Example code where this is an issue?

Benchmark before/after ?

Comment by Mike Anderson [ 09/Dec/15 9:08 PM ]

Hi alex, I'm trying to improve the fast path for reflection, this will be a bunch of changes, together with clj-1784 and a few other ideas I have.

I don't think it will be productive to have an extensive debate / benchmarking over every single change. Would it be better to make a new ticket for all changes together with benchmarking for the overall impact?

Happy to do this, but it would be helpful if you could give an indication that this stuff will be accepted on the assumption that an overall improvement is demonstrated. I don't want to waste effort on Clojure dev if you guys are not interested in performance improvements in this space. And I don't have time to have an extensive debate over every individual change.

Comment by Alex Miller [ 10/Dec/15 7:51 AM ]

Each ticket should identify a specific problem and propose a solution with evidence that it helps. If there are multiple issues it is quite likely that they may move at different rates.

If you identify a hot spot, then we are happy to look at it. But we have to start from a problem to solve not just: "here are some changes". If the change makes things better, then you should be able to demonstrate that.

Comment by Alex Miller [ 10/Dec/15 7:53 AM ]

I would say that I am still dubious that the right answer to a problem with reflection is not just removing the reflection. But I can't evaluate that until you provide a problem.

Comment by Mike Anderson [ 10/Dec/15 7:10 PM ]

The problem is that it is inefficient (and therefore slow for users of reflection), as stated in the description. If you have an alternative way that you would like to see it worded, can you suggest?

Whether or not people should be using reflection is orthogonal to making reflection itself faster. I agree people should use type hints where they can but plenty of people don't have time to figure it out / don't know how to do properly / don't realise it is happening so surely any improvement for these cases should be welcome?

Also you haven't answered my question: would you like everything rolled into a single reflection performance improvement ticket, or not?

Comment by Alex Miller [ 11/Dec/15 10:11 AM ]

The title of this ticket is "Optimise argument boxing during reflection". That is a solution, not a problem. What I'm looking for is a title like "Reflection with boxed args is slow" and a description that starts with some example code demonstrating the problem. (That example code often makes for a particularly good template for a test that should be in the patch as well.)

I am then looking for evidence that the change you are suggesting improves the problem. For a performance issue, I am specifically looking for a before/after benchmark, preferably using a testing tool like criterium that gives me some confidence that the gains are real.

From a prioritization standpoint, I do not consider reflection performance to be a high priority because the best answer is probably: don't use reflection. That said, I'm willing to consider it, particularly if there is a compelling example where it may be difficult to remove the reflection or where it is particularly non-obvious that the reflection is happening.

Regarding your final question, we prefer to consider individual problems rather "a big bunch of changes", so separate would be better.





[CLJ-1827] Reflection warning introduced in CLJ-1259 Created: 13/Oct/15  Updated: 27/Oct/15  Resolved: 27/Oct/15

Status: Closed
Project: Clojure
Component/s: None
Affects Version/s: Release 1.8
Fix Version/s: Release 1.8

Type: Defect Priority: Major
Reporter: Alex Miller Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: print, reflection, regression
Environment:

1.8.0-beta1


Attachments: Text File clj-1827-v1.patch    
Patch: Code
Approval: Ok

 Description   

The patch for CLJ-1259 addressed the reflection warnings in pprint. However, a different ticket introduced some new code in between when this patch was written and applied and thus there is a new reflection warning produced in the Clojure build:

[java] Reflection warning, clojure/pprint/pretty_writer.clj:419:12 - call to method write can't be resolved (target class is unknown).

This ticket is to address that one newly introduced case to remove the warning.

Patch: clj-1827-v1.patch
Screened by: Alex Miller



 Comments   
Comment by Andy Fingerhut [ 15/Oct/15 10:23 AM ]

Patch clj-1827-v1.patch dated Oct 15 2015 eliminates the one reflection warning in pretty_writer.clj.





[CLJ-1774] Field access on typed record does not preserve type Created: 02/Jul/15  Updated: 03/Jul/15

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

Type: Enhancement Priority: Major
Reporter: Michael Blume Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: defrecord, reflection, typehints


 Description   
(ns field-test.core
  (:import [java.util UUID]))

(defrecord UUIDWrapper [^UUID uuid])

(defn unwrap [^UUIDWrapper w]
  (.-uuid w)) ; <- No reflection

(defn get-lower-bits [^UUIDWrapper w]
  (-> w .-uuid .getLeastSignificantBits)) ; <- Reflection :(

The compiler seems to have all the information it needs, but lein check prints

Reflection warning, field_test/core.clj:10:3 - reference to field getLeastSignificantBits on java.lang.Object can't be resolved.

(test case also at https://github.com/MichaelBlume/field-test)



 Comments   
Comment by Alex Miller [ 02/Jul/15 4:31 PM ]

afaik, that ^UUID type hint on the record field doesn't do anything. The record field will be of type Object (only ^double and ^long affect the actual field type).

Perhaps more importantly, it is bad form to use Java interop to retrieve the field values of a record. Keyword access for that is preferred.

Comment by Nicola Mometto [ 03/Jul/15 4:48 AM ]

The same issue applies for deftypes where keyword access is not an option

Comment by Alex Miller [ 03/Jul/15 12:17 PM ]

Per http://clojure.org/datatypes: "You should always program to protocols or interfaces -
datatypes cannot expose methods not in their protocols or interfaces"

Along those lines, usually for deftypes, I gen an interface with the proper types if necessary, then have the deftype implement the interface to expose the field.

Also per http://clojure.org/datatypes:

"note that currently a type hint of a non-primitive type will not be used to constrain the field type nor the constructor arg, but will be used to optimize its use in the class methods" - that is, inside a method implemented on the record/type, referring to the field should have the right hint. So in the example above, if unwrap was an interface or protocol implementation method on the record, and you referred to the field, you should expect the hint to be utilized in that scenario.

So, my contention would be that all of the behavior described in this ticket should be expected based on the design, which is why I've reclassified this as an enhancement, not a defect.





[CLJ-1688] Object instance members should resolve to Object Created: 30/Mar/15  Updated: 30/Mar/15

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

Type: Enhancement Priority: Minor
Reporter: Michael Blume Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: reflection, typehints


 Description   
(defn unparse-pattern ^String [pattern] (.toString pattern))
Reflection warning, ring/swagger/coerce.clj:22:41 - reference to field toString can't be resolved.

Reflection isn't really necessary here, we could just special-case the methods on Object.






[CLJ-1687] Clojure doesn't resolve static calls even when it has all information needed to do so Created: 30/Mar/15  Updated: 30/Mar/15

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

Type: Enhancement Priority: Minor
Reporter: Michael Blume Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: reflection, typehints


 Description   

If I create a class with two methods, one of which takes (String, String), and the other taking (String, Number), and then write a function

(defn foo
  [x ^String y]
  (Thing/hello x y))

it seems obvious that I'm trying to call the first method and not the second. But on lein check, clojure prints

Reflection warning, resolve_fail/core.clj:6:3 - call to static method hello on resolve_fail.Thing can't be resolved (argument types: unknown, java.lang.String).

unless I also type-hint x.



 Comments   
Comment by Nicola Mometto [ 30/Mar/15 2:32 PM ]

I have looked at this countless times while working on tools.analyzer and hacking the reflector and found out that there doesn't seem to be a way to make things like this "work" without breaking other cases





[CLJ-1664] Inconsistency in overflow-handling between type-hinted and reflective calls Created: 19/Feb/15  Updated: 19/Feb/15

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

Type: Defect Priority: Minor
Reporter: Michael Blume Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: numerics, reflection


 Description   
(import 'java.io.DataOutputStream)
(import 'java.io.ByteArrayOutputStream)

(defn- ->bytes
  "Convert a Java primitive to its byte representation."
  [write v]
  (let [output-stream (ByteArrayOutputStream.)
        data-output (DataOutputStream. output-stream)]
    (write data-output v)
    (seq (.toByteArray output-stream))))

(defn int->bytes [n]
  (->bytes 
    #(.writeInt ^DataOutputStream %1 %2)
    n))

(defn int->bytes-ref [n]
  (->bytes 
    #(.writeInt %1 %2)
    n))

user=> (int->bytes 5)
(0 0 0 5)
user=> (int->bytes-ref 5)
(0 0 0 5)
user=> (int->bytes (inc Integer/MAX_VALUE))

IllegalArgumentException Value out of range for int: 2147483648  clojure.lang.RT.intCast (RT.java:1115)
user=> (int->bytes-ref (inc Integer/MAX_VALUE))
(-128 0 0 0)

So it looks like type-hinting the DataOutputStream results in bytecode calling RT.intCast, which throws because the value is too large. In the reflective case, we locate the method writeInt at runtime, and then do not call RT.intCast, but instead allow the long to be downcast without bounds checking.

It seems like we should be calling RT.intCast in both cases?






[CLJ-938] Output of clojure.reflect is not suitable for type hints Created: 23/Feb/12  Updated: 19/Jul/15

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

Type: Enhancement Priority: Minor
Reporter: Brandon Bloom Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: reflection, typehints


 Description   

I'm trying to write a macro which generate reify forms using some reflection.

clojure.reflect produces type symbols similar to 'java.util.Iterator<>

Unfortunately, the compiler doesn't accept the <> syntax in type hints:

(reify Iterable (^{:tag java.util.Iterator} iterator [this] nil)) ; works

(reify Iterable (^{:tag java.util.Iterator<>} iterator [this] nil)) ;; fails
CompilerException java.lang.RuntimeException: java.lang.ClassNotFoundException: java.util.Iterator<>, compiling:(NO_SOURCE_PATH:1325)

It seems like the compiler should understand the <> just enough to strip it, rather than reject it. This would make it much easier to write correct macros involving type hinting and reflection.

The workaround I have been using is:

(defn hint
"clojure.reflect demarks generics with angle brackets, but
the compiler does not support generics in type hints"
[obj tag]
(let [tag (-> tag .toString (.replace "<>" "") symbol)]
(with-meta obj {:tag tag}))



 Comments   
Comment by Brandon Bloom [ 24/Feb/12 1:37 AM ]

I'm sorry, I got this wrong earlier. The problem is real, but it's arrays, not generics.

My workaround is useless... :-/





[CLJ-910] [Patch] Allow for type-hinting the method receiver in memfn Created: 13/Jan/12  Updated: 01/Sep/12  Resolved: 01/Sep/12

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

Type: Enhancement Priority: Minor
Reporter: Tassilo Horn Assignee: Unassigned
Resolution: Completed Votes: 1
Labels: performance, reflection

Attachments: Text File 0001-Make-memfn-allow-for-type-hinting-the-method-receive.patch    
Patch: Code
Approval: Ok

 Description   

The attached patch copies metadata given to the method name symbol of memfn to the method receiver in the expansion. That way, memfn is able to be used even for type-hinted calls resulting in a big performance win.

user> (time (dotimes [i 1000000] ((memfn intValue) 1)))
Reflection warning, NO_SOURCE_FILE:1 - call to intValue can't be resolved.
"Elapsed time: 2579.229115 msecs"
nil
user> (time (dotimes [i 1000000] ((memfn ^Number intValue) 1)))
"Elapsed time: 12.015235 msecs"
nil


 Comments   
Comment by Tassilo Horn [ 08/Mar/12 3:56 AM ]

Updated patch.

Comment by Aaron Bedra [ 21/Aug/12 6:06 PM ]

Applies properly against d4170e65d001c8c2976f1bd7159484056b9a9d6d. Things look good to me.





Generated at Fri Sep 30 05:15:57 CDT 2016 using JIRA 4.4#649-r158309.