<< Back to previous view

[CLJ-1259] Remove reflection in pprint and cl-format to improve performance Created: 09/Sep/13  Updated: 17/Oct/15  Resolved: 12/Oct/15

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

Type: Enhancement Priority: Major
Reporter: Andy Fingerhut Assignee: Unassigned
Resolution: Completed Votes: 1
Labels: ft, performance, print

Attachments: Text File clj-1259-1.txt     Text File clj-1259-2.patch    
Patch: Code
Approval: Ok


There are many occurrences of reflection in the pprint implementation.

By eliminating all of them, I ran one benchmark of pprint'ing a Clojure map that resulted in a 300 Kbyte output. After eliminating reflection, the elapsed time to pprint was reduced by 18% (about 14.0 sec down to about 11.5 sec) on a recent model MacBook Pro.

Patch: clj-1259-2.patch
Screened by: Alex Miller

Comment by Andy Fingerhut [ 09/Sep/13 11:36 PM ]

Patch clj-1259-1.txt eliminates all occurrences of reflection in pprint, and all files loaded from pprint.clj. It also sets warn-on-reflection to true for those files, in hopes of making it more obvious if a new use of reflection is added there.

Comment by Alex Miller [ 09/Oct/15 8:59 AM ]

Added -2 patch that changes commit message and adds diff context, no semantic changes, attribution retained.

Comment by Michael Blume [ 12/Oct/15 5:14 PM ]

Added v3 patch which fixes one more bit of reflection around line 419, and moves a few type hints to the return types of helpers so they don't have to be duplicated. Kept expanded context, attribution unchanged.

Comment by Alex Miller [ 12/Oct/15 7:58 PM ]

Michael, the -2 patch is the one that has been screened and ok'ed by Rich - any other change will need to go into a new ticket. I've removed the -v3 patch to reduce confusion when Stu goes to apply.

Comment by Andy Fingerhut [ 12/Oct/15 11:07 PM ]

Michael, looks like that reflection warning is in code introduced after I wrote my patch. Now that the v2 patch has been committed, since it includes setting warn-on-reflection to true for the pretty_writer.clj source file, there is now that one reflection warning printed during builds of the latest master. Eliminating that warning might be enough motivation for the Clojure core team to get a fix for it into a later 1.8.0-beta, but of course that is their decision.

Comment by Alex Miller [ 13/Oct/15 8:18 AM ]

Please create a new ticket for the new warning...

Comment by Andy Fingerhut [ 13/Oct/15 2:11 PM ]

Michael, would you be interested in creating the ticket and creating the patch for it? If not, I can get to it in a week or two.

Comment by Alex Miller [ 13/Oct/15 4:56 PM ]

I created CLJ-1827 to track this - feel free to drop a patch there.

Comment by Andy Fingerhut [ 17/Oct/15 6:41 AM ]

Michael, I have attached a minimal patch to CLJ-1827 for the one reflection warning still in pretty_writer.clj.

I did not save a copy of your patch before Alex removed it from this ticket, so cannot create a patch similar to yours for CLJ-1827. If you attach your patch as is to CLJ-1827, I could look into updating it for latest master.

Generated at Thu Nov 26 11:47:51 CST 2015 using JIRA 4.4#649-r158309.