Bug in Utils.tryWith

[Aleksandr Dubinsky]

Hi Aleksey,

With all due respect to your hard work and achievement, I hope you realize
you're coming across as a bit unfriendly to your users.

Not giving your users a crucial error message because "you just need to get
better [at something else]" is not friendly.

Being woefully mistaken about "any hassle with building the actual hsdis
can be resolved with a tiny bit of following up" is not friendly. (Have you
tried following any of the several bits of advice on building hsdis? Sure,
there's lots of instructions. The trouble is none of them work. I wasted a
whole day. The killer is you need experience in native development on Linux
to make sense of and fix the errors.)

Hiding your options in the source code is not friendly either. Fact is you
talk about `jmh.perfasm.savePerf` in the WinPerfAsmProfiler javadoc without
mentioning `jmh.perfasm.savePerfTo` and `jmh.perfasm.savePerfToFile`, or
even how to use savePerf (hint: it takes a boolean). Taken as an omission,
I would understand it (I don't write perfect docs either!). But as a
principled stand, no. That's just not friendly to your users.

Lastly, requiring users to bundle their benchmarks in a jar and execute
them from the command-line is a bit unfriendly as well. That said, I was
happy to discover that I am able to include `jmh-core` and
`jmh-generator-annprocess` as `<scope>test</scope>` and call Runner inside
a JUnit test from the IDE with no difference in results. For me, that is
the most convenient usage and best way to organize my code. You seem to
warn against that. Are there concrete reasons? AFAIK, the benchmarks launch
their own virgin VMs which isolates them from the launch environment. (To
make a valid point: having all of the permutations of my benchmark launch
options inside my code inside my VCS is a big benefit. Being able to launch
benchmarks using all the infrastructure built for running tests, such as CI
severs, IDEs, etc, is another.)

I am sorry for coming across hostile. JMH is a great library not just for
micro-benchmarks but for benchmarking and profiling in general. (Finally,
line-by-line profiling of JIT'd code!) Perfasm makes it a real winner. I
hope it goes down in history as one among the great Java libraries. JMH is
very user friendly in most ways, and I hope that can be improved further.
I'll do my part by participating in StackOverflow to catch whatever falls
through the cracks in the documentation.

-- Aleksandr Dubinsky


On Tue, May 26, 2015 at 2:34 AM, Aleksey Shipilev <
aleksey.shipilev at oracle.com> wrote:

> Hi,
> Hi,
>
> On 05/24/2015 05:40 PM, Aleksandr Dubinsky wrote:
> > Firstly, I must say, you are very brave to be using raw Runtime.exec! In
> > any case, there is a simple bug at line 340. `if (err > 0)` should be `if
> > (err != 0)`.
>
> Yes, thanks, fixed:
>  https://bugs.openjdk.java.net/browse/CODETOOLS-7901434
>
> We should probably switch to ProcessBuilders as well.
>
> > I would also modify WinPerfAsmProfiler line 161 to say "Failed to start
> > xperf: " + errs + "\nEnsure that you are running this profiling session
> as
> > an administrator." or similar.
>
> Nope. That line is the part of sanity testing, assuming we have checked
> the profiler is available. We "just" need to get better at figuring out
> if xperf is available under current privileges, see
> WinPerfAsmProfiler::<clinit>.
>
>
> > There is something hugely difficult in trying to get HSDIS for Windows,
> > with copyright nazis and other idiots getting in the way. It's good that
> > you link to the Oracle wiki, although it isn't very helpful. This
> > StackOverflow discussion (to which I've contributed the best I could) is
> > more helpful
> >
> http://stackoverflow.com/questions/1503479/how-to-see-jit-compiled-code-in-jvm/15146962#15146962
> > (and expands on the same solutions that are found in the comments to the
> > Oracle wiki). Might make sense linking here until the Oracle wiki
> improves,
> > you write a dedicated help page, or someone just publishes a windows
> binary
> > that works.
>
> Nope again. The link points to the PrintAssembly description, and any
> hassle with building the actual hsdis can be resolved with a tiny bit of
> following up from there and/or Googling.
>
>
> > Lastly, I beg you to document all of the system properties referenced in
> > WinPerfAsmProfiler AND AbstractPerfAsmProfiler as a TABLE in the
> > WinPerfAsmProfiler javadoc comment.
>
> You have to understand these options are not stable, and cannot be a
> part of documentation. You use these options at your own risk, and
> should consult with the (authoritative) source code to make sure it does
> what you want. Not every tuning knob is going to be documented.
>
> The only UX improvement we can do is to make profilers accept these
> options through regular means, e.g. -prof perfasm:events=cycles,insns;
> which will allow, among other things, using -prof perfasm:help.
>
> Thanks,
> -Aleksey
>
>