+1!

Nicely done.

> On 04 Feb 2015, at 13:49, Hannes Wallnoefer <[email protected]> 
> wrote:
> 
> I uploaded a new wevrev:
> 
> http://cr.openjdk.java.net/~hannesw/8062141/webrev.02/
> 
> This version builds the property maps and value containers directly in the 
> JSON parser, making use of PropertyMap's history feature to reuse maps.
> 
> Answers to Marcus' questions are inline below.
> 
> Am 2015-01-31 um 16:40 schrieb Marcus Lagergren:
>> Awesome numbers, Hannes. Why was the last JSON parser so slow? I guess it 
>> was just a case of clean room implement per spec to ensure correctness, 
>> right? I don’t think we’ve paid a lot of attention to the JSON code since it 
>> was first written.
> 
> I couldn't find a single issue with the old parser, so it's more like a death 
> from thousand cuts. There's certainly room for improvement, which is 
> important as these are the same base classes used by our main parser.
>> 
>> Does everything compile without warnings? I notice you have explicitly added 
>> a @SuppressWarnings(“unchecked”) in JSONFunctions. Why was this the case? 
>> List<?> still too weak?
> You're right, that @SuppressWarnings was not needed. In any case I removed 
> the code that used it in my rewrite :)
> 
>> Do your ProperyHashMap changes give us benefits in performance elsewhere 
>> too? (or slowdowns for that matter)
> I did these changes while working on the JSONParser and they seemed to help. 
> However, to be sure I wrote a PropertyHashMap microbenchmark and what I found 
> is that while these changes make creation of PropertyHashMap slightly faster, 
> they slightly slow down access of properties. Since the latter is probably 
> more important in a longer running process I removed those changes from my 
> new webrev.
> 
>> Do you have any micro benchmarks you used to test this to check in ?
> 
> I added one, it's in test/examples/json-parser-micro.js
> 
> Hannes
> 
>> 
>> +1
>> 
>> /M
>> 
>>> On 30 Jan 2015, at 17:40, Hannes Wallnoefer<[email protected]>  
>>> wrote
>>> 
>>> Please review JDK-8062141: Various performance issues parsing JSON:
>>> 
>>> http://cr.openjdk.java.net/~hannesw/8062141/
>>> 
>>> Thew new JSON parser is about 2-3 x faster than our old one, roughly on par 
>>> with the ones in Rhino or V8.
>>> 
>>> Hannes
> 

Reply via email to