search

leeoo / spymemcached

Automatically exported from code.google.com/p/spymemcached
0 stars 0 forks source link

Performance problem in StringUtils.isJsonObject #298

Open GoogleCodeExporter opened 9 years ago

GoogleCodeExporter commented 9 years ago 
Version 2.10.5 (as used by the couchdb-akka-extension)

I've noticed a substantial slowdown in the StringUtils.isJsonObject which uses 
'new Integer(s)/catch NumberFormatException'. In my case, the amount of 
exceptions being thrown from this method severely impact the performance. I've 
tried a local patch which uses the code from Commons-lang 
(NumberUtils.isNumber), and it literally triples my throughput, while also 
lowering the latency.

Original issue reported on code.google.com by philip.l...@gmail.com on 25 Feb 2014 at 9:06

GoogleCodeExporter commented 9 years ago 
thanks for reporting this.. can you give me some actual numbers and the payload 
you did use? Or is this not related to the actual payload (size, value..)

I think we can change the implementation as long as we keep the semantics.
Also, if you could point me to your benchmarking code that would be great

Original comment by michael....@gmail.com on 26 Feb 2014 at 5:35

GoogleCodeExporter commented 9 years ago 
The benchmark used was a blackbox load test (so no micro benchmark, sorry) 
using wrk with settings '-t30 -c400 -d300s’ that was hitting a Spray+Akka 
endpoint URL. The code queries a view with a child key to retrieve an inverse 
relationship linking a child key to a parent key. The retrieved payload was 
extremely minimal:

{{{
{"total_rows”:10000,"rows":[
  {"id”:”<parent_key>","key”:”<child_key>","value”:”<parent_key>"}
  ]
}
}}}

The load test was run several times; before the patch, a maximum of ~600 
requests per second could be achieved with CPU usage on my cores at about 60%. 
After this change, it was up to ~1900 requests per second, with 90% CPU usage.

The code was analysed with Yourkit, which showed a hotspot in 
StringUtils.isJsonObject for the reasons mentioned above. I created a patch 
using Apache Commons-lang3, got it to compile using the ant+ivy script, but 
then found out there was also an old version of commons-lang already included 
in the project. Funnily enough I could remove my added dependency, but 
afterwards the build process went haywire, and no matter what I did, the build 
would complain it couldn’t find the commons-lang dependency. It might have 
been my IvyIDEA plugin going a bit crazy though.

Since I haven’t used Ant (nor Ivy) for many years, I figured I’d just add a 
ticket describing my findings, rather than a full blown patch + tests. My 
apologies for that.

Original comment by philip.l...@gmail.com on 26 Feb 2014 at 8:31

GoogleCodeExporter commented 9 years ago 
Hi Philip, no worries - thanks!

So you are storing and loading JSON documents, is that correct (since you use 
views)?

Cheers,
Michael

Original comment by michael....@gmail.com on 26 Feb 2014 at 8:38

GoogleCodeExporter commented 9 years ago 
I'm asking because looking at the code, it looks like you are not storing a 
JSON like {}? otherwise the new Integer would never be triggered (nevertheless 
we should fix it but I'd like to repro :))

Original comment by michael....@gmail.com on 26 Feb 2014 at 8:41

GoogleCodeExporter commented 9 years ago 
Hi Michael,

You’re right - I’m trying to figure out exactly why I’m descending that 
deep into the method. I’ll report back in a few moments.

Cheers,

Phil

Original comment by philip.l...@gmail.com on 26 Feb 2014 at 8:44

GoogleCodeExporter commented 9 years ago 
Alright, got it. Sorry for the confusion (and my misunderstanding of when the 
cascade into StringUtils happens). The problem is indeed not the response; but 
rather the query request. I'm using the following:

val query: Query = new Query()
query.setKey(child_key)
query.setLimit(1)

Which I then use to query the retrieved view. And what do you know, it's 
actually the limit on the query that was calling the isJsonObject() method, and 
causing the performance slowdown. If I remove it, the problem is gone.

HTH,

Phil

Original comment by philip.l...@gmail.com on 26 Feb 2014 at 9:34

GoogleCodeExporter commented 9 years ago 
Ah!

can you please raise an issue here in the couchbase bug tracker? We'll take it 
from there :)

http://www.couchbase.com/issues/browse/JCBC

Original comment by michael....@gmail.com on 26 Feb 2014 at 9:44

GoogleCodeExporter commented 9 years ago

Original comment by michael....@gmail.com on 26 Feb 2014 at 9:44

GoogleCodeExporter commented 9 years ago 
Hold on - shouldn't this still be fixed in here as well? Regardless of the 
cause, it seems to be a good place for an improvement, no?

Original comment by philip.l...@gmail.com on 26 Feb 2014 at 9:47

GoogleCodeExporter commented 9 years ago 
Hmm you are right, let's keep this in mind for future improvement :)

Original comment by michael....@gmail.com on 26 Feb 2014 at 9:49

GoogleCodeExporter commented 9 years ago 
Alright, looking forward to the next release ;-)

Original comment by philip.l...@gmail.com on 26 Feb 2014 at 9:50

GoogleCodeExporter commented 9 years ago 
Also, there already is an issue in the couchbase tracker for this issue:

https://www.couchbase.com/issues/browse/SPY-127

Should I create a new issue in JCBC?

Original comment by philip.l...@gmail.com on 26 Feb 2014 at 9:52

GoogleCodeExporter commented 9 years ago 
yes please one in JCBC for the Query issue itself

Original comment by michael....@gmail.com on 26 Feb 2014 at 9:59

GoogleCodeExporter commented 9 years ago 
Done: https://www.couchbase.com/issues/browse/JCBC-421

Original comment by philip.l...@gmail.com on 26 Feb 2014 at 10:31