Open to contributions ?

[yagonax]

Hi,

I am currently setting up a gradle project to use jmh, and before writing mine, I bumped into your plugin. However out of the box there are a couple of reasons which might prevent me from using it, but I'd be glad to contribute if you are ok with the ideas.

1/ source set) What if I have a perf sourceSet that I'd like to use instead of creating the jmh one (hence giving me the ability to customize the source set layout) ? Wouldn't that be a good candidate for a plugin extension, to specify default source set config when none is given ?

2/ task type) It seems that only a single jmh task to run benchmarks is created with the plugin. Wouldn't it be more flexible to create a task type (JmhRunTask for example) that extends JavaExec and expose it directly so that people can create a task with this type themselves ?

3/ config per task) Wouldn't it be convenient to use a configuration object per task to use the extension to configure each task (thus separating task config from plugin config), for those who don't want to create a task explicitely with JmhRunTask type for example.

4/ shadow jar) Following article http://gvsmirnov.ru/blog/tech/2014/03/10/keeping-your-benchmarks-separate.html, I would also find very convenient to be able to expose a jmh jar with batteries included to be able to deploy that easily on a benchmark server for instance without the need for gradle there.

5/ flexible run config) It is indeed convenient to get the type safety when configuring a jmh run task, but it seems to me like you are reverse engineering the option parser spec configured inside the plugin extension. Not only does it seem like a lot of work, but it might be hard to be in sync with the latest jmh public features. Thus I would define the JmhRunTask with basic config options (string array), and then provide 3 different configuration objects in the config : a) the beautiful way as you did, with type safety, and nice human readable name, with an extra parameter to pass a list of options to solve the 'not yet implemented in the plugin' problem, b) the flexible mapping, where a human readable mapping is defined in the plugin config (with default mapping defined inside the plugin), and then each task can use those human readable name, which will be converted to short names when building the arg array. It also supports a raw array for those who already like and know the jmh cmdline syntax and don't want to translate it when running on a remote server with shadow jar.

6/ test classes in jar) Sometimes, performance tests share classes with unit tests, and it might be convenient to be able to create a jar with the test outputs.

7/ jmh task group) It is possible in gradle to assign tasks to a given group. Adding tasks to a 'jmh' group here would make things easier to understand for the users (and provide nice locality when running 'gradle tasks')

8/ task desc) Add a description to each task that wich mentions the jmh command line associated with it (to enable copy paste when using shadow jar)

9/ defaults) Define default values at the plugin level which would be applied while building the arg list, should no other value exist

Since an example is worth a 1000 words, here is an example of what I would expect from a jmh plugin configuration after implementing those ideas:

jmh {
    sourceSet = sourceSets.perf
    tasks {
        run3 {
            include = '.*HelloWorld.*'
            threads = 1
            iterations = 1
            warmupIterations = 1
            rawArgs = ['-f 1']
        }
    }
    argNameMappings = [       
        bs : 'batchSize',
        wbs: 'warmupBatchSize',
        i  : ['measurementIterationsNumber','iterations'],  // multiple alias support
        t  : 'threads',
        wi : 'warmupIterations' 
    ]
    argDefaults = [
        i  : 1,
        bm : 'thrpt'
    ]
    mtasks {
        run4 {
            include = ".*HelloWorld.*"
            kvArgs = [
                threads:1,
                wi:1 // support values no defined in mapping (for forward compat among other things)
            ]
            rawArgs = "-f 1" 
        }
        run5 {
            rawArgs = [".*HelloWorld.*", "-t","1","-i", "1", "-wi", "1", "-f", "1"]
        }
        run6 {
            description = "some other desc"
            rawArgs = ".*HelloWorld.* -t 1 -i 1 -wi 1 -f 1"
        }
    }
}

task jmhRun1(type: JmhRunTask, dependsOn: jmhClasses) {
    args = [".*HelloWorld.*", "-t","1","-i", "1", "-wi", "1", "-f", "1"]
}

task jmhRun2(type: JmhRunTask, dependsOn: jmhClasses) {
    args = ".*HelloWorld.* -t 1 -i 1 -wi 1 -f 1"
}

Example of what if would look like in a terminal:

> gradle tasks
...

Jmh tasks
---------
jmhJar - create the jar containing micro benchmark classes, test classes, production classes
jmhJarAll - create a shadow jar containing micro benchmark the output of jmhJar and all its dependencies. Main class is Jmh's Main
jmhRun1 - run jmh. args='.*HelloWorld.* -t 1 -i 1 -wi 1 -f 1'
jmhRun2 - run jmh. args='.*HelloWorld.* -t 1 -i 1 -wi 1 -f 1'
jmhRun3 - run jmh. args='.*HelloWorld.* -t 1 -i 1 -wi 1 -f 1'
jmhRun4 - run jmh. args='.*HelloWorld.* -t 1 -i 1 -wi 1 -f 1 -mb thrpt'
jmhRun5 - run jmh. args='.*HelloWorld.* -t 1 -i 1 -wi 1 -f 1'
jmhRun6 - some other desc. args='.*HelloWorld.* -t 1 -i 1 -wi 1 -f 1'
...

Would you be open to contributions ? If so which of the previous elements would you consider as potential changes to the current plugin ?

Cheers

Aurélien

[yagonax]

Hi Cédric,

Did you get a chance to take a look at my previous message ? If so, I would really appreciate it if you could just give me your feedback, so that I can move on, either rolling-out my own by forking this one, or contributing to this one directly.

Cheers

Aurélien

[melix]

Hi and thank you for your patience. First of all, sure, the plugin is open for contributions. Now let's try to answer your points:

1/ source set) What if I have a perf sourceSet that I'd like to use instead of creating the jmh one (hence giving me the ability to customize the source set layout) ? Wouldn't that be a good candidate for a plugin extension, to specify default source set config when none is given ?

Adding a sourceSet for jmh should be as easy as doing:

sourceSets.jmh.java.srcDir 'path/to/my/sources'

It doesn't seem to be very "Gradle idiomatic" to have something which behaves differently whether there are source files inside the source set or not. It's IMHO better to explicitly declare a new one.

2/ task type) It seems that only a single jmh task to run benchmarks is created with the plugin. Wouldn't it be more flexible to create a task type (JmhRunTask for example) that extends JavaExec and expose it directly so that people can create a task with this type themselves ?

It would be a nice improvement, yes.

3/ config per task) Wouldn't it be convenient to use a configuration object per task to use the extension to configure each task (thus separating task config from plugin config), for those who don't want to create a task explicitely with JmhRunTask type for example.

In general, I prefer to have either a task based plugin or a convention based one. As soon as we introduce a task, I would prefer to drop the extension altogether.

4/ shadow jar) Following article http://gvsmirnov.ru/blog/tech/2014/03/10/keeping-your-benchmarks-separate.html, I would also find very convenient to be able to expose a jmh jar with batteries included to be able to deploy that easily on a benchmark server for instance without the need for gradle there.

Agreed, but I wouldn't make this plugin dependent on the shadow plugin if possible.

5/ flexible run config)

+1. I indeed "reverse engineered" the options, and it's fragile. Still very convenient, so +1 for having both options.

6/ test classes in jar)

I think it's already doable, by adding elements to the jmh task classpath.

7/ jmh task group) 8/ task desc) +1

9/ defaults)

Makes sense, even though as I said, I prefer to avoid extensions in that case.

Thanks a lot for the input. If you plan to add this, it would be nice to have separate issues/PRs for each feature.

[yagonax]

Thanks for your comprehensive reply. Now I'm the one not being that reactive :) A couple of stuff kept me busy over the last few weeks, but I'll eventually contribute, once I get some free bandwidth. Cheers