Proposed javac argument processing performance improvement

[David Schlosnagle]

On Fri, Jun 17, 2011 at 12:03 AM, Jonathan Gibbons
<jonathan.gibbons at oracle.com> wrote:
> Dave,
>
> Thanks for the update, and for looking at making your patch work even
> faster.
>
> For a perf-related change like this, no new regression tests will be
> necessary.
> The normal rules in a case like this are to make sure that nothing is
> broken,
> meaning that all the (langtools) tests should still pass and that it should
> still
> be capable to build the OpenJDK forest. Normally, it is sufficient to do a
> regular
> build of OpenJDK; in some circumstances, it would be expected to pass a
> build with SKIP_BOOT_CYCLE=false, meaning that it should be able to build
> OpenJDK -- and the result should itself be capable of building OpenJDK. This
> double build verifies that the bits from the first build are good enough to
> function as a BOOT_JDK.  But, I don't think the double build is necessary in
> this case,
> since this is a relatively simple change related to command line processing.
>
> Normally, all the langtools tests should pass all time time, on all
> supported
> platforms (Linux, Windows, Solaris.) I haven't checked out the Mac port
> recently
> to see how the tests perform on that platform.
>
> -- Jon

Here's the updated patch using Iterable<File> throughout, and the
results are similar to the previous patch. Let me know if there is
anything else I can do to get this through the process.

On an aside, I tried to keep the IllegalArgumentException thrown in
JavacTaskImpl consistent with the previous message; however, the code
is much messier now  (this just begs for something like Guava's
Joiner) and I'd prefer using something simpler along the lines of
throw new IllegalArgumentException("Malformed arguments " +
filenames);. That would have the realistic effect of moving from a
space delimited list of filename arguments to a comma and space
delimited and surrounded by square brackets per
AbstractCollection#toString().

I've included timings for several different size real world modules
compiled with javac on Windows XP SP2 (where I had seen the worst
performance to start). All times are in milliseconds. With the
changes, processArgs is consistently sub-second, and linear in time
relative to the number of input files.

        |        Before       |         After       |          Delta        |
# files | processArgs | javac | processArgs | javac | processArgs | javac   |
--------+-------------+-------+-------------+-------+-------------+---------+
   5522 |   16170.863 | 48163 |     280.484 | 37087 |  -15890.379 |  -11075 |
   4436 |    9498.519 | 61043 |     200.735 | 49452 |   -9297.784 |  -11591 |
    268 |      57.270 |  5775 |      16.923 |  5641 |     -40.347 |    -134 |
    154 |      28.571 |  3147 |      11.322 |  2985 |     -17.249 |    -161 |
    101 |      14.486 | 40621 |       7.814 | 40214 |      -6.672 |    -407 |
     73 |      12.277 | 12180 |       6.677 | 12125 |      -5.600 |     -55 |
      8 |       1.695 |  1718 |       1.309 |  1643 |      -0.386 |     -74 |
      4 |       1.500 |  2098 |       1.399 |  2000 |      -0.101 |     -98 |
      1 |       1.035 |  1442 |       0.911 |  1294 |      -0.124 |    -148 |


Thanks,
- Dave
-------------- next part --------------
A non-text attachment was scrubbed...
Name: javac-iterable-args.patch
Type: application/octet-stream
Size: 6864 bytes
Desc: not available
Url : http://mail.openjdk.java.net/pipermail/compiler-dev/attachments/20110621/e531406f/javac-iterable-args.patch