tools.reader

Some uses of reflection in tools.reader code slow it down unnecessarily

Details

  • Type: Enhancement Enhancement
  • Status: Closed Closed
  • Priority: Minor Minor
  • Resolution: Completed
  • Affects Version/s: None
  • Fix Version/s: None
  • Component/s: None
  • Labels:
    None
  • Patch:
    Code

Description

Attached patch uses type hints to eliminate several instances of reflection in the tools.reader code.

FYI, you can run 'lein check' to cause Leiningen to compile the code with warn-on-reflection true, I believe for every source file (not sure about the tests, but reflection isn't such a big deal in them anyway).

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
Nicola Mometto added a comment -

Andy, I don't see any patch attached to this ticket, I think you forgot to attach it.

(P.S. thanks, I didn't know about 'lein check')

Show
Nicola Mometto added a comment - Andy, I don't see any patch attached to this ticket, I think you forgot to attach it. (P.S. thanks, I didn't know about 'lein check')
Hide
Permalink
Andy Fingerhut added a comment -

Patch trdr-6-eliminate-reflection-with-type-hints-patch-v1.txt dated Apr 13 2013 eliminates all occurrences of reflection found in latest version of tools.reader. Please check them carefully before committing them, especially the ones in default_data_readers.clj.

And I know the reflection warnings in default_data_readers.clj exist in Clojure's code, too, where you copied those from. CLJ-1080 has a patch addressing those and many other reflections within Clojure's code.

Show
Andy Fingerhut added a comment - Patch trdr-6-eliminate-reflection-with-type-hints-patch-v1.txt dated Apr 13 2013 eliminates all occurrences of reflection found in latest version of tools.reader. Please check them carefully before committing them, especially the ones in default_data_readers.clj. And I know the reflection warnings in default_data_readers.clj exist in Clojure's code, too, where you copied those from. CLJ-1080 has a patch addressing those and many other reflections within Clojure's code.
Hide
Permalink
Nicola Mometto added a comment - - edited

Andy, casting to char makes tools.reader crash under clojure-1.3, apparently casting to char is possible only from clojure 1.4.

Could you please submit a patch removing reflection in default_data_readers.clj and the docstring fixes only while I try to figure out a way to maintain clojure 1.3 compatibility and remove the reflection? (or if you have an idea on how to do it, you're more than welcome )

EDIT:
I edited your patch manually, and pushed a commit to type hint to char only for >clojure-1.3.0
I don't know if there is a way to avoid the reflection from clojure 1.3.0, but for the all the other versions of clojure all reflection is gone, thanks!

Show
Nicola Mometto added a comment - - edited Andy, casting to char makes tools.reader crash under clojure-1.3, apparently casting to char is possible only from clojure 1.4. Could you please submit a patch removing reflection in default_data_readers.clj and the docstring fixes only while I try to figure out a way to maintain clojure 1.3 compatibility and remove the reflection? (or if you have an idea on how to do it, you're more than welcome ) EDIT: I edited your patch manually, and pushed a commit to type hint to char only for >clojure-1.3.0 I don't know if there is a way to avoid the reflection from clojure 1.3.0, but for the all the other versions of clojure all reflection is gone, thanks!
Hide
Permalink
Andy Fingerhut added a comment -

Great. I was off-line there for a while, but glad you noticed the 1.3 compatibility issue where I didn't, and glad you found a different way to eliminate reflection with 1.4 and later.

Show
Andy Fingerhut added a comment - Great. I was off-line there for a while, but glad you noticed the 1.3 compatibility issue where I didn't, and glad you found a different way to eliminate reflection with 1.4 and later.

People

Vote (0)
Watch (0)

Dates

  • Created:
    Updated:
    Resolved:
Atlassian JIRA (v4.4#649-r158309) | Report a problem
Powered by a free Atlassian JIRA open source license for Clojure. Try JIRA - bug tracking software for your team.