Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for running benchmarks with WebClient #142

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

AbhyudayaSharma
Copy link
Member

Adds support for running benchmarks using WebClient.

@jenkinsci/gsoc2019-role-strategy

Copy link
Member

@oleg-nenashev oleg-nenashev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be need to do some refactoring to avoid need in inheriting JenkinsRule and de-facto having two wrappers inside every performance test. How closely is WebClient tied to JenkinsRule, would we be able to make it a static class from it (Maybe a new class name so that we retain binary compatibility)

@AbhyudayaSharma
Copy link
Member Author

AbhyudayaSharma commented Jun 17, 2019

How closely is WebClient tied to JenkinsRule

From a quick look at the code, it is possible to extract WebClient from JenkinsRule. WebClient basically depends on the reference to the the JUnit test Description, Jenkins instance, it's URL and a private method related to CrumbIssuer. @oleg-nenashev Do you recommend to extract it out of JenkinsRule? Also, in the above test benchmark, we directly instantiate the WebClient just because it is running on a single thread. In role strategy, when we were running it on two threads, we had to create a separate state because it is not thread-safe.

https://github.com/jenkinsci/role-strategy-plugin/blob/master/src/test/java/jmh/benchmarks/WebClientBenchmark.java#L21-L29

From our benchmarks, the only thing I see that is different is the URL because the port it runs is different every time. Please let me know what you think about it.

@AbhyudayaSharma
Copy link
Member Author

AbhyudayaSharma commented Jun 17, 2019

Or we could just have another utility class that creates a WebClient for each thread in the benchmark and the overall benchmark class could look something like:

@JmhBenchmark
public class SomeBenchmark {
    public static class State1 extends JmhJenkinsRule {}
    
    public static class ThreadState extends WebClientThreadState {}
    
    @Benchmark
    public void benchmark(State1 state1, ThreadState state2) {
        state2.webClient.goTo("");
    }
}

@oleg-nenashev
Copy link
Member

Let's park it for a while I will release #141 without it, so we won't need to rush this PR. Also, a merge conflict

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants