RFR: JDK-8150736: Excessive disk space used by build system

Tue Oct 4 11:26:23 UTC 2016

On 2016-10-04 13:20, Erik Joelsson wrote:
> New webrev: http://cr.openjdk.java.net/~erikj/8150736/webrev.02/

Looks great. Ship it! :)

/Magnus
>
>
> On 2016-10-04 11:03, Magnus Ihse Bursie wrote:
>> On 2016-10-03 10:22, Erik Joelsson wrote:
>>> Hello,
>>>
>>>
>>> On 2016-10-01 14:13, David Holmes wrote:
>>>> Hi Erik,
>>>>
>>>> This is a big change that is hard to digest. I assume the actual 
>>>> images themselves are unaffected by this change?
>>>>
>>> I understand that and I have been working on this for some time in 
>>> the background. Yes, I have done quite extensive comparisons during 
>>> this work so there should not be any relevant differences in the 
>>> images. There are some, for example jexec and jspawnhelper are now 
>>> stripped while the old strip logic missed them. I consider that 
>>> fixing a bug.
>>>
>>> I was about to leave this patch for another release but since the 
>>> schedule was moved, and Magnus is available to review things again, 
>>> I thought it was worth trying to get it in. I believe it will add 
>>> significant value.
>>
>> I believe so too. :-) It's great to see this cleared up!
>>
>> Overall, I think your patch looks good. A few things:
>>
>> * In hotspot/make/GensrcJvmti.gmk, you have added out-commented code. 
>> :-)
>>
> Created bug JDK-8167078 to track this issue and referred to it in the 
> comments, both in GensrcJvmti.gmk and Copy-java.base.gmk. I don't want 
> to throw away the build logic for these header files since I believe 
> the correct fix is to get the files from hotspot.
>> * You have missed updating copyright year overall.
>>
> Fixed.
>> * I'm wondering a bit about this comment about hotspot, that is now 
>> removed:
>>     # NOTE: The old build did not strip binaries on macosx.
>> As far as I can see, we are now stripping libjvm.dylib. Is this 
>> something that has changed, or was the old comment incorrect?
>>
> The old comment was correct for the hotspot build, but then the images 
> build did a second round of strips. This is how the JDK libraries used 
> to get stripped. Since I now let SetupNativeCompilation do the 
> stripping, the secondary strip logic is removed and macosx libjvm must 
> be stripped like all the others.
>> * In jdk/make/Import.gmk, we did a lot of special handling for 
>> libjsig, e.g. handle STATIC_BUILD separately, or selecting different 
>> versions depending on if client or server was built (which even 
>> seemed to be OS dependent). While the old logic looked really 
>> suspicios, I'm not sure how to match that to the new code in 
>> CompileLibjsig.gmk. Are the end result really the same? If not, can 
>> you specify how things have changed and reassure me that it is 
>> correct? :-)
>>
> Short answer: The end result is the same.
>
> STATIC_BUILD disabled building and importing of libjsig and still does.
>
> What the old logic basically did was creating symlinks from each 
> variant subdir like this:
>
> lib/amd64/libjsig.so
> lib/amd64/server/libjsig.so -> ../libjsig.so
>
> For the library that was trivial, but it also did the same for the 
> symbol files, and zipped the link if zipping symbols. On macosx, with 
> the dSYM dirs, this got even more complicated, which is the reason for 
> platform checks (as well as libjsig not being built on windows).
>
> The only difference is this. When --with-native-debug-symbols=zipped 
> is set, we end up with both zipped and non zipped debug symbols in the 
> modules_libs/<module> dir. Because of this, both the zipped and non 
> zipped symlink are created, where before, only the zipped one would be.
>>>> An initial nit - why use "product-" in the target names? 
>>>> "images-jdk" etc seems perfectly sufficient, and "product" may be a 
>>>> misnomer.
>>>>
>>> A while back when we started introducing other images (test-image, 
>>> docs-image, symbols-image) we also renamed the old images target to 
>>> product-images. We left the alias images though since it's fairly 
>>> well established, and introduced the all-images target to build all 
>>> of them. To me it was then natural to create sub targets of 
>>> product-images rather than images for jdk and jre. I do think the 
>>> new targets are too long however and because of that will likely not 
>>> see much use. Perhaps we can add aliases for them too as you suggest.
>>
>> I agree that product-images-jdk is not a very good name. But I also 
>> agree with Erik's reasoning. :-) However, I have a suggestion that I 
>> think will suit you both. :)
>>
>> I suggest that we call the new, fine-grained, targets "jdk-image", 
>> "jre-image" and "symbols-image". This is in line with the singular 
>> form of "docs-image" and "test-image". The actual reason that 
>> "images" (and hence, "product-images") was using the plural form was 
>> that it consisted of these several individual images. So, if we have:
>>
>> # Define product images
>> product-images: jdk-image jre-image symbols-image
>>
>> # Add a common alias
>> images: product-images
>>
>> In fact, we almost already does something like this:
>>
>> # This target builds the product images, e.g. the JRE and JDK image
>> # (and possibly other, more specific versions)
>> product-images: product-images-jdk product-images-jre 
>> product-images-symbols \
>>     zip-security exploded-image
>>
> This is so simple and obvious, fixed.
>>
>> ... which got me wondering if "zip-security" is actually correctly 
>> placed. I assume it should be a dependency to product-images-jdk (or 
>> rather "jdk-image" :)), instead.
>>
> zip-security is a bit of an oddity. Right now is not a good time to 
> sort it out however. I have other work in progress where it will be. 
> Added a comment about it.
>
> /Erik
>> /Magnus
>>
>>
>>> /Erik
>>>> Thanks,
>>>> David
>>>>
>>>> On 30/09/2016 8:17 PM, Erik Joelsson wrote:
>>>>> Here is a rather large patch which should make life better for most
>>>>> people building and developing OpenJDK. The main goal of this 
>>>>> patch is
>>>>> to reduce unnecessary space used by the build. A side goal also 
>>>>> become
>>>>> to figure out a better general strategy for handling native debug 
>>>>> info
>>>>> during the build since that was a large part of the wasted space. 
>>>>> These
>>>>> are the high level changes:
>>>>>
>>>>> 1. Removed the dist step in the hotspot build (and the corresponding
>>>>>    import step in the jdk build) and linking libjvm directly into
>>>>>    support/modules_libs. This removes 2 extra copies of all the 
>>>>> hotspot
>>>>>    native libraries.
>>>>> 2. Made many files in the exploded image ($(OUTPUT_DIR)/jdk) symlinks
>>>>>    into support/modules_*. Note that binaries and libraries cannot be
>>>>>    symlinks since the image won't run then. Debug symbols and other
>>>>>    files seem to work fine though.
>>>>> 3. Introduced separate top level targets for building the jdk, jre 
>>>>> (and
>>>>>    serverjre) images. A developer can run "make 
>>>>> product-images-jdk" and
>>>>>    skip the others. The test targets now depend on just the jdk image
>>>>>    to avoid building extra images that aren't needed for tests.
>>>>> 4. Changed SetupNativeCompilation to strip binaries by default unless
>>>>>    told not to, and removed the separate strip build step. This 
>>>>> lets us
>>>>>    get rid of modules_*-stripped.
>>>>> 5. Build debug symbols into the OUTPUT_DIR in SetupNativeCompilation
>>>>>    (instead of OBJECT_DIR and then copy to OUTPUT_DIR). For normal 
>>>>> jdk
>>>>>    and hotspot libraries, this means the debug symbols end up 
>>>>> directly
>>>>>    in support/modules_libs/$MODULE/... and there is only one copy 
>>>>> of them.
>>>>> 6. If zipping debug symbols, this will also leave the unzipped
>>>>>    debugsymbols in OUTPUT_DIR, where we wouldn't do so before. This
>>>>>    also means that the exploded image will get symlinks to the 
>>>>> unzipped
>>>>>    debug symbols regardless of if we are asked to zip them or not. 
>>>>> This
>>>>>    in turn means that debugging the exploded image will just work. 
>>>>> This
>>>>>    change is space neutral since we were already keeping a copy of 
>>>>> the
>>>>>    unzipped debugsymbols in the OBJECT_DIR before, they have just 
>>>>> been
>>>>>    moved to a more useful place.
>>>>> 7. Stop copying debug symbols from test native binaries since we 
>>>>> aren't
>>>>>    stripping them anyway. Better to just leave debug information 
>>>>> in the
>>>>>    test binaries.
>>>>>
>>>>> The reduction in size of course varies a lot depending on OS and
>>>>> configuration, but as an example, on my 64bit Linux workstation
>>>>> (debug-symbols=zipped) the size of the build directory has gone from
>>>>> 5.8GB when building "make images" before to 3.8GB when building "make
>>>>> product-images-jdk". The biggest gain is from only building the jdk
>>>>> image when only that is needed. A fairer comparison is "make images
>>>>> test-image" which has gone from 7.3GB to 5.4GB, much due to not
>>>>> duplicating debug information in test binaries. On Solaris the 
>>>>> gain is
>>>>> generally bigger.
>>>>>
>>>>> Another nice effect of this is that the exploded image now has debug
>>>>> information for all native libraries by default, and that handling of
>>>>> dSYM directories on Macosx should now work properly.
>>>>>
>>>>> The only significant changes detected by comparison builds is that 
>>>>> some
>>>>> binaries that previously weren't stripped, now are, which they also
>>>>> should be.
>>>>>
>>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8150736
>>>>>
>>>>> Webrev: http://cr.openjdk.java.net/~erikj/8150736/webrev.01/
>>>>>
>>>>> /Erik
>>>>>
>>>
>>
>