Fast keyword intern

Question

Keyword's intern(Symbol) method uses recursive invocation to get a valid keyword instance.I think it can be rewrite into a 'for loop'
to reduce method invocation cost.
So i developed this patch, and make some simple benchmark.Run the following command line three times after 'ant jar':

java -Xms64m -Xmx64m -cp test:clojure.jar clojure.main -e "(time (dotimes (link: n 10000000) (keyword (str n))))"

Before patched:

"Elapsed time: 27343.827 msecs"
"Elapsed time: 26172.653 msecs"
"Elapsed time: 25673.764 msecs"

After patched:

"Elapsed time: 24884.142 msecs"
"Elapsed time: 23933.423 msecs"
"Elapsed time: 25382.783 msecs"

It looks the patch make keyword's intern a little more fast.

The patch is attached and test.

Thanks.

P.S. I've signed the contributor agreement, and my email is killme2008@gmail.com .

Answer 1

Comment made by: alexmiller

Looks intriguing (and would be a nice change imo). I ran this on a json parsing benchmark I used for the keyword changes and saw ~3% improvement.

Answer 2

Comment made by: killme2008

Updated the patch, remove the 'k == null' clause in for loop,it's not necessary.

Answer 3

Comment made by: jafingerhut

Dennis, while JIRA can handle multiple patches with the same name, it can be confusing for people discussing the patches, and for some scripts I have to evaluate them. Please consider giving the patches different names (e.g. with version numbers in them), or removing older ones if they are obsolete.

Answer 4

Comment made by: killme2008

Hi,andy

Thank you for reminding me.I deleted the old patch.

Answer 5

Comment made by: killme2008

I am glad to see it is helpful.I benchmark the patch with current master branch,it's fine too.

Answer 6

Comment made by: killme2008

Is this patch can be merged? Or is it rejected?

Answer 7

Comment made by: alexmiller

As a minor enhancement, this patch has not yet been high enough priority to be considered yet.

Answer 8

Comment made by: killme2008

All right.Hope to merge it.Thanks.

Answer 9

Reference: https://clojure.atlassian.net/browse/CLJ-1493 (reported by killme2008)