You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@jclouds.apache.org by Niraj Tolia <no...@github.com> on 2013/09/27 02:33:39 UTC
[jclouds] Reduce reflection overhead by caching results of
Invokable.getParameters... (#155)
...()
Improves request signing performance (GETs and PUTs) for S3 by >
3X. These performance problems were seen in production and diagnosed
using the YourKit profiler.
You can merge this Pull Request by running:
git pull https://github.com/ntolia/jclouds annotations
Or you can view, comment on it, or merge it online at:
https://github.com/jclouds/jclouds/pull/155
-- Commit Summary --
* Reduce reflection overhead by caching results of Invokable.getParameters()
-- File Changes --
M apis/s3/src/main/java/org/jclouds/s3/util/S3Utils.java (8)
M core/src/main/java/org/jclouds/rest/InputParamValidator.java (11)
M core/src/test/java/org/jclouds/rest/InputParamValidatorTest.java (2)
-- Patch Links --
https://github.com/jclouds/jclouds/pull/155.patch
https://github.com/jclouds/jclouds/pull/155.diff
Re: [jclouds] Reduce reflection overhead by caching results of
Invokable.getParameters... (#155)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds #437](https://buildhive.cloudbees.com/job/jclouds/job/jclouds/437/) FAILURE
Looks like there's a problem with this pull request
[(what's this?)](https://www.cloudbees.com/what-is-buildhive)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/155#issuecomment-25214899
Re: [jclouds] Reduce reflection overhead by caching results of
Invokable.getParameters... (#155)
Posted by Adrian Cole <no...@github.com>.
The approach I'm referring to is isolating reflection ops into a parser. Instead of doing reflection at the edges like this, there would be metadata that shows the position and context of the param. I use this approach un feign which has negligible and constant (microsecond) overhead per invocation. We can do something similar in jclouds for spots like this.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/155#issuecomment-25216185
Re: [jclouds] Reduce reflection overhead by caching results of
Invokable.getParameters... (#155)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-java-7-pull-requests #704](https://jclouds.ci.cloudbees.com/job/jclouds-java-7-pull-requests/704/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/155#issuecomment-25216201
Re: [jclouds] Reduce reflection overhead by caching results of
Invokable.getParameters... (#155)
Posted by Niraj Tolia <no...@github.com>.
@adriancole I am pretty sure there are better ways to do this but, unfortunately, I am not familiar enough with the code and how reflection is used here to implement what you are suggesting.
FWIW, here are the performance improvements I see on my local machine. I can also run numbers against master if needed.
Experiment: Generate 100K signed urls to warm the JVM and then generate 100K more and time the second batch.
signGetBlob(): Generates 1,823 URLs/sec with 1.6.3-SNAPSHOT (44e8487) and 5798 URL/s with this patch.
signPutBlob(): Generates 1,274 URLs/sec with 1.6.3-SNAPSHOT (44e8487) and 4,093 URL/s with this patch.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/155#issuecomment-25216880
Re: [jclouds] Reduce reflection overhead by caching results of
Invokable.getParameters... (#155)
Posted by Adrian Cole <no...@github.com>.
Ps I bet this approach could be cleaned up in general, as it seems a bit too lazy. Would be nice to isolate in caliper (I can create a project for this if folks want)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/155#issuecomment-25215941
Re: [jclouds] Reduce reflection overhead by caching results of
Invokable.getParameters... (#155)
Posted by Adrian Cole <no...@github.com>.
Nice catch. There's probably a few places like this.
+1
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/155#issuecomment-25215067
Re: [jclouds] Reduce reflection overhead by caching results of
Invokable.getParameters... (#155)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests #244](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/244/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/155#issuecomment-25225440
Re: [jclouds] Reduce reflection overhead by caching results of
Invokable.getParameters... (#155)
Posted by Niraj Tolia <no...@github.com>.
@adriancole Done, done, and done. Should we tweak contributing-to-jclouds.markdown in the jclouds-site project to make this clear for new contributors?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/155#issuecomment-25225166
Re: [jclouds] Reduce reflection overhead by caching results of
Invokable.getParameters... (#155)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-java-7-pull-requests #705](https://jclouds.ci.cloudbees.com/job/jclouds-java-7-pull-requests/705/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/155#issuecomment-25225447
Re: [jclouds] Reduce reflection overhead by caching results of
Invokable.getParameters... (#155)
Posted by Andrew Phillips <no...@github.com>.
> I use this approach un feign which has negligible and constant (microsecond) overhead per invocation.
@adriancole Link please? ;-)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/155#issuecomment-25216277
Re: [jclouds] Reduce reflection overhead by caching results of
Invokable.getParameters... (#155)
Posted by Adrian Cole <no...@github.com>.
after I finish up things, I'll help merge my old gist into the main docs.
https://gist.github.com/adriancole/5592785
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/155#issuecomment-25250291
Re: [jclouds] Reduce reflection overhead by caching results of
Invokable.getParameters... (#155)
Posted by Adrian Cole <no...@github.com>.
ps. I think process-wise, we're to make a jira on this, reword your commit to be prefixed by that, and note this pull request in the jira.
then, any of us can cherry-pick it in.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/155#issuecomment-25218997
Re: [jclouds] Reduce reflection overhead by caching results of
Invokable.getParameters... (#155)
Posted by Niraj Tolia <no...@github.com>.
Somehow I messed things up while pushing to github. Will push a fix soon. The "correct" patch locally does pass mvn test.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/155#issuecomment-25215158
Re: [jclouds] Reduce reflection overhead by caching results of
Invokable.getParameters... (#155)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-java-7-pull-requests #703](https://jclouds.ci.cloudbees.com/job/jclouds-java-7-pull-requests/703/) FAILURE
Looks like there's a problem with this pull request
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/155#issuecomment-25214886
Re: [jclouds] Reduce reflection overhead by caching results of
Invokable.getParameters... (#155)
Posted by Adrian Cole <no...@github.com>.
thx for publishing the results. My aside was a larger issue than this PR,
so no worries.
+1 to merge this big improvement.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/155#issuecomment-25218808
Re: [jclouds] Reduce reflection overhead by caching results of
Invokable.getParameters... (#155)
Posted by Adrian Cole <no...@github.com>.
https://github.com/adriancole/playground-caliper
On Sep 26, 2013 6:16 PM, "Andrew Phillips" <no...@github.com> wrote:
> I use this approach un feign which has negligible and constant
> (microsecond) overhead per invocation.
>
> @adriancole <https://github.com/adriancole> Link please? ;-)
>
> —
> Reply to this email directly or view it on GitHub<https://github.com/jclouds/jclouds/pull/155#issuecomment-25216277>
> .
>
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/155#issuecomment-25216319
Re: [jclouds] Reduce reflection overhead by caching results of
Invokable.getParameters... (#155)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests #243](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/243/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/155#issuecomment-25216197
Re: [jclouds] Reduce reflection overhead by caching results of
Invokable.getParameters... (#155)
Posted by Adrian Cole <no...@github.com>.
master
```
commit e8ef5c06657319c22a307a45fe0f9aeeb94e37aa
Author: Niraj Tolia <nt...@maginatics.com>
Date: Thu Sep 26 17:27:28 2013 -0700
[JCLOUDS-301] Reduce reflection overhead of Invokable.getParameters()
By caching the results from Invokable.getParameters(), this commit
improves request signing performance (GETs and PUTs) for S3 by >
3X. These performance problems were seen in production and diagnosed
using the YourKit profiler.
```
1.6.x
```
commit 82708c98de6eae74ccf53cf9b5415aafec6ff662
Author: Niraj Tolia <nt...@maginatics.com>
Date: Thu Sep 26 17:27:28 2013 -0700
[JCLOUDS-301] Reduce reflection overhead of Invokable.getParameters()
By caching the results from Invokable.getParameters(), this commit
improves request signing performance (GETs and PUTs) for S3 by >
3X. These performance problems were seen in production and diagnosed
using the YourKit profiler.
```
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/155#issuecomment-25250715
Re: [jclouds] Reduce reflection overhead by caching results of
Invokable.getParameters... (#155)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests #242](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/242/) FAILURE
Looks like there's a problem with this pull request
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/155#issuecomment-25214915