Implement key conversions with symbol by Java within Date._strptime #4676
Conversation
| parser = org.jruby.util.RubyDateParser.new | ||
| map = parser.parse(JRuby.runtime.current_context, fmt, str) | ||
| return map.nil? ? nil : map.to_hash.inject({}){|hash,(k,v)| hash[k.to_sym] = v; hash} | ||
| return JRuby.runtime.current_context.getRubyDateParser.parseAndCreateHash(JRuby.runtime.current_context, fmt, str) |
There was a problem hiding this comment.
quite an ugly design to get current_context to pass it current_context. maybe this whole method should be/delegate to a native ext impl.
There was a problem hiding this comment.
@muga I think you could make parser.parse just return the symboled hash right? Then @kares comment about currentContext (and having to know there is a cache) can be encapsulated by the parse.
As a broader comment. A Bag is made then a Map is made and then a Hash with Symbols is made. I have not viewed how much a Bag modifies its data but I wondered if you could just drop the bag and the map and just use a Hash with symbols directly as you parse? I can see some post processing happening at end of bag and when map is made but I just wondered about whether this could just be one thing or not.
To improve performance of Java strptime implemented on #4635, this PR fixes
Date._strptimeto implement key conversions with symbol in Java. Because It reduces the count of objects passing between JRuby and Java.a RubyDateParser object in ThreadContext to reduce the cost of RubyDateParser object creations. The cache is basically similar approach to RubyDateFormatter in ThreadContext. And this PR assumes #4675.
The following is my micro benchmark based on https://github.com/macournoyer/tinyrb. The performance by this PR is 2.5 times faster maximumly.
The raw test data: https://gist.github.com/muga/e566299fc9afdaa0511a8bb7804d5a7f