[DISCUSS] Benchmarks module package structure

[Nick Reich <nr...@pivotal.io>]

Team,

I am in the progress of adding new benchmarks to the (currently sparse)
geode-benchmarks module. The lack of current examples in the module leads
me to wonder what the correct organization of benchmarks in the module is
(and if this is the right location).

The existing benchmarks are all in org.apache.geode.cache.benchmark.
Following this pattern would (presumably) result in benchmark subpackages
in each package that has benchmarks. Making the root package
org.apache.geode.benchmark would remove this proliferation of sub packages.
However, both these approaches have the issue that package level
methods/classes cannot be accessed from benchmarks as they will never share
a package with the production code.

1) Should benchmarks then not be put in special benchmark packages?

2) Should our benchmarks not invoke package level methods/classes in the
case that we should use benchmark packages? Or should such benchmarks not
reside in the benchmarks module?

3) Is geode-benchmarks where we intend all benchmarks, only certain classes
of benchmarks (all using jmh for example), or would we prefer embedding
them in the modules where the code being benchmarked resides?

Thanks for your input.

Re: [DISCUSS] Benchmarks module package structure

[Kirk Lund <kl...@apache.org>]

+1 for adding src/jmh/java to each module. I submitted a PR with that
change and a new benchmark in geode-core last year, but I ended up closing
it after a couple months. If anyone wants help, let me know.

On Mon, Jan 8, 2018 at 3:23 PM, Nick Reich <nr...@pivotal.io> wrote:

> I think if we can make it work, removing the benchmarks module and moving
> the benchmarks into the module where the code they are testing resides
> would be the ideal. Talking with Dan, this would result in a src/jmh/java
> source directory in any module with benchmarks, which I think would result
> in a good level of proximity between the benchmarks and the code they are
> testing.
>
> On Mon, Jan 8, 2018 at 3:15 PM, Kirk Lund <kl...@apache.org> wrote:
>
> > We'll probably end up writing benchmarks for classes or methods that are
> > package-private, so that would be one reason to not create a special
> > benchmarks package.
> >
> > On Mon, Jan 8, 2018 at 1:40 PM, Dan Smith <ds...@pivotal.io> wrote:
> >
> > > I think this module was specifically added for running JMH benchmarks
> > since
> > > it's pulling in the JMH plugin. JMH is framework for easily writing
> > > microbenchmarks.
> > >
> > > I think it makes sense to put JMH benchmarks in the same package as the
> > > code under tests, since you may end up writing a microbenchmark for
> just
> > > one in internal class. Arguably, all of the modules could pull in the
> JMH
> > > plugin and these benchmarks could sit in those modules. I think the
> > current
> > > structure might have just been the easiest way to integrate JMH into
> the
> > > build.
> > >
> > > -Dan
> > >
> > > On Sun, Jan 7, 2018 at 9:59 PM, Xiaojian Zhou <gz...@pivotal.io>
> wrote:
> > >
> > > > The package might be always a problem. Even you put the cq benchmark
> > code
> > > > under geode-cq to near its source code, it might still have to access
> > > code
> > > > under other package, such as geode-core.
> > > >
> > > > So I think put benchmark test code under benchmark package is ok.
> Your
> > > > option 2) is good.
> > > >
> > > > Regards
> > > > Gester
> > > >
> > > > On Fri, Jan 5, 2018 at 11:57 AM, Nick Reich <nr...@pivotal.io>
> wrote:
> > > >
> > > > > Team,
> > > > >
> > > > > I am in the progress of adding new benchmarks to the (currently
> > sparse)
> > > > > geode-benchmarks module. The lack of current examples in the module
> > > leads
> > > > > me to wonder what the correct organization of benchmarks in the
> > module
> > > is
> > > > > (and if this is the right location).
> > > > >
> > > > > The existing benchmarks are all in org.apache.geode.cache.
> benchmark.
> > > > > Following this pattern would (presumably) result in benchmark
> > > subpackages
> > > > > in each package that has benchmarks. Making the root package
> > > > > org.apache.geode.benchmark would remove this proliferation of sub
> > > > packages.
> > > > > However, both these approaches have the issue that package level
> > > > > methods/classes cannot be accessed from benchmarks as they will
> never
> > > > share
> > > > > a package with the production code.
> > > > >
> > > > > 1) Should benchmarks then not be put in special benchmark packages?
> > > > >
> > > > > 2) Should our benchmarks not invoke package level methods/classes
> in
> > > the
> > > > > case that we should use benchmark packages? Or should such
> benchmarks
> > > not
> > > > > reside in the benchmarks module?
> > > > >
> > > > > 3) Is geode-benchmarks where we intend all benchmarks, only certain
> > > > classes
> > > > > of benchmarks (all using jmh for example), or would we prefer
> > embedding
> > > > > them in the modules where the code being benchmarked resides?
> > > > >
> > > > > Thanks for your input.
> > > > >
> > > >
> > >
> >
>

Re: [DISCUSS] Benchmarks module package structure

[Nick Reich <nr...@pivotal.io>]

I think if we can make it work, removing the benchmarks module and moving
the benchmarks into the module where the code they are testing resides
would be the ideal. Talking with Dan, this would result in a src/jmh/java
source directory in any module with benchmarks, which I think would result
in a good level of proximity between the benchmarks and the code they are
testing.

On Mon, Jan 8, 2018 at 3:15 PM, Kirk Lund <kl...@apache.org> wrote:

> We'll probably end up writing benchmarks for classes or methods that are
> package-private, so that would be one reason to not create a special
> benchmarks package.
>
> On Mon, Jan 8, 2018 at 1:40 PM, Dan Smith <ds...@pivotal.io> wrote:
>
> > I think this module was specifically added for running JMH benchmarks
> since
> > it's pulling in the JMH plugin. JMH is framework for easily writing
> > microbenchmarks.
> >
> > I think it makes sense to put JMH benchmarks in the same package as the
> > code under tests, since you may end up writing a microbenchmark for just
> > one in internal class. Arguably, all of the modules could pull in the JMH
> > plugin and these benchmarks could sit in those modules. I think the
> current
> > structure might have just been the easiest way to integrate JMH into the
> > build.
> >
> > -Dan
> >
> > On Sun, Jan 7, 2018 at 9:59 PM, Xiaojian Zhou <gz...@pivotal.io> wrote:
> >
> > > The package might be always a problem. Even you put the cq benchmark
> code
> > > under geode-cq to near its source code, it might still have to access
> > code
> > > under other package, such as geode-core.
> > >
> > > So I think put benchmark test code under benchmark package is ok. Your
> > > option 2) is good.
> > >
> > > Regards
> > > Gester
> > >
> > > On Fri, Jan 5, 2018 at 11:57 AM, Nick Reich <nr...@pivotal.io> wrote:
> > >
> > > > Team,
> > > >
> > > > I am in the progress of adding new benchmarks to the (currently
> sparse)
> > > > geode-benchmarks module. The lack of current examples in the module
> > leads
> > > > me to wonder what the correct organization of benchmarks in the
> module
> > is
> > > > (and if this is the right location).
> > > >
> > > > The existing benchmarks are all in org.apache.geode.cache.benchmark.
> > > > Following this pattern would (presumably) result in benchmark
> > subpackages
> > > > in each package that has benchmarks. Making the root package
> > > > org.apache.geode.benchmark would remove this proliferation of sub
> > > packages.
> > > > However, both these approaches have the issue that package level
> > > > methods/classes cannot be accessed from benchmarks as they will never
> > > share
> > > > a package with the production code.
> > > >
> > > > 1) Should benchmarks then not be put in special benchmark packages?
> > > >
> > > > 2) Should our benchmarks not invoke package level methods/classes in
> > the
> > > > case that we should use benchmark packages? Or should such benchmarks
> > not
> > > > reside in the benchmarks module?
> > > >
> > > > 3) Is geode-benchmarks where we intend all benchmarks, only certain
> > > classes
> > > > of benchmarks (all using jmh for example), or would we prefer
> embedding
> > > > them in the modules where the code being benchmarked resides?
> > > >
> > > > Thanks for your input.
> > > >
> > >
> >
>

Re: [DISCUSS] Benchmarks module package structure

[Kirk Lund <kl...@apache.org>]

We'll probably end up writing benchmarks for classes or methods that are
package-private, so that would be one reason to not create a special
benchmarks package.

On Mon, Jan 8, 2018 at 1:40 PM, Dan Smith <ds...@pivotal.io> wrote:

> I think this module was specifically added for running JMH benchmarks since
> it's pulling in the JMH plugin. JMH is framework for easily writing
> microbenchmarks.
>
> I think it makes sense to put JMH benchmarks in the same package as the
> code under tests, since you may end up writing a microbenchmark for just
> one in internal class. Arguably, all of the modules could pull in the JMH
> plugin and these benchmarks could sit in those modules. I think the current
> structure might have just been the easiest way to integrate JMH into the
> build.
>
> -Dan
>
> On Sun, Jan 7, 2018 at 9:59 PM, Xiaojian Zhou <gz...@pivotal.io> wrote:
>
> > The package might be always a problem. Even you put the cq benchmark code
> > under geode-cq to near its source code, it might still have to access
> code
> > under other package, such as geode-core.
> >
> > So I think put benchmark test code under benchmark package is ok. Your
> > option 2) is good.
> >
> > Regards
> > Gester
> >
> > On Fri, Jan 5, 2018 at 11:57 AM, Nick Reich <nr...@pivotal.io> wrote:
> >
> > > Team,
> > >
> > > I am in the progress of adding new benchmarks to the (currently sparse)
> > > geode-benchmarks module. The lack of current examples in the module
> leads
> > > me to wonder what the correct organization of benchmarks in the module
> is
> > > (and if this is the right location).
> > >
> > > The existing benchmarks are all in org.apache.geode.cache.benchmark.
> > > Following this pattern would (presumably) result in benchmark
> subpackages
> > > in each package that has benchmarks. Making the root package
> > > org.apache.geode.benchmark would remove this proliferation of sub
> > packages.
> > > However, both these approaches have the issue that package level
> > > methods/classes cannot be accessed from benchmarks as they will never
> > share
> > > a package with the production code.
> > >
> > > 1) Should benchmarks then not be put in special benchmark packages?
> > >
> > > 2) Should our benchmarks not invoke package level methods/classes in
> the
> > > case that we should use benchmark packages? Or should such benchmarks
> not
> > > reside in the benchmarks module?
> > >
> > > 3) Is geode-benchmarks where we intend all benchmarks, only certain
> > classes
> > > of benchmarks (all using jmh for example), or would we prefer embedding
> > > them in the modules where the code being benchmarked resides?
> > >
> > > Thanks for your input.
> > >
> >
>

Re: [DISCUSS] Benchmarks module package structure

[Dan Smith <ds...@pivotal.io>]

I think this module was specifically added for running JMH benchmarks since
it's pulling in the JMH plugin. JMH is framework for easily writing
microbenchmarks.

I think it makes sense to put JMH benchmarks in the same package as the
code under tests, since you may end up writing a microbenchmark for just
one in internal class. Arguably, all of the modules could pull in the JMH
plugin and these benchmarks could sit in those modules. I think the current
structure might have just been the easiest way to integrate JMH into the
build.

-Dan

On Sun, Jan 7, 2018 at 9:59 PM, Xiaojian Zhou <gz...@pivotal.io> wrote:

> The package might be always a problem. Even you put the cq benchmark code
> under geode-cq to near its source code, it might still have to access code
> under other package, such as geode-core.
>
> So I think put benchmark test code under benchmark package is ok. Your
> option 2) is good.
>
> Regards
> Gester
>
> On Fri, Jan 5, 2018 at 11:57 AM, Nick Reich <nr...@pivotal.io> wrote:
>
> > Team,
> >
> > I am in the progress of adding new benchmarks to the (currently sparse)
> > geode-benchmarks module. The lack of current examples in the module leads
> > me to wonder what the correct organization of benchmarks in the module is
> > (and if this is the right location).
> >
> > The existing benchmarks are all in org.apache.geode.cache.benchmark.
> > Following this pattern would (presumably) result in benchmark subpackages
> > in each package that has benchmarks. Making the root package
> > org.apache.geode.benchmark would remove this proliferation of sub
> packages.
> > However, both these approaches have the issue that package level
> > methods/classes cannot be accessed from benchmarks as they will never
> share
> > a package with the production code.
> >
> > 1) Should benchmarks then not be put in special benchmark packages?
> >
> > 2) Should our benchmarks not invoke package level methods/classes in the
> > case that we should use benchmark packages? Or should such benchmarks not
> > reside in the benchmarks module?
> >
> > 3) Is geode-benchmarks where we intend all benchmarks, only certain
> classes
> > of benchmarks (all using jmh for example), or would we prefer embedding
> > them in the modules where the code being benchmarked resides?
> >
> > Thanks for your input.
> >
>

Re: [DISCUSS] Benchmarks module package structure

[Xiaojian Zhou <gz...@pivotal.io>]

The package might be always a problem. Even you put the cq benchmark code
under geode-cq to near its source code, it might still have to access code
under other package, such as geode-core.

So I think put benchmark test code under benchmark package is ok. Your
option 2) is good.

Regards
Gester

On Fri, Jan 5, 2018 at 11:57 AM, Nick Reich <nr...@pivotal.io> wrote:

> Team,
>
> I am in the progress of adding new benchmarks to the (currently sparse)
> geode-benchmarks module. The lack of current examples in the module leads
> me to wonder what the correct organization of benchmarks in the module is
> (and if this is the right location).
>
> The existing benchmarks are all in org.apache.geode.cache.benchmark.
> Following this pattern would (presumably) result in benchmark subpackages
> in each package that has benchmarks. Making the root package
> org.apache.geode.benchmark would remove this proliferation of sub packages.
> However, both these approaches have the issue that package level
> methods/classes cannot be accessed from benchmarks as they will never share
> a package with the production code.
>
> 1) Should benchmarks then not be put in special benchmark packages?
>
> 2) Should our benchmarks not invoke package level methods/classes in the
> case that we should use benchmark packages? Or should such benchmarks not
> reside in the benchmarks module?
>
> 3) Is geode-benchmarks where we intend all benchmarks, only certain classes
> of benchmarks (all using jmh for example), or would we prefer embedding
> them in the modules where the code being benchmarked resides?
>
> Thanks for your input.
>