Request for Review (XXL): 6989984: Use standard include model for Hotspot

[Dmitry Samersoff]

Volker,

I guess we should finally get rid of src/share/vm/precompiled.hpp but 
let it be a separate fix.

-Dmitry

On 2010-11-19 22:01, Stefan Karlsson wrote:
> Hi Volker,
>
> On 2010-11-19 19:02, Volker Simonis wrote:
>> Hi Stefan and Bengt,
>>
>> I have some comments/questions regarding your change:
>>
>> - how did you create the "src/share/vm/precompiled.hpp". I have the
>> impression that it contains much fewer lines compared to the old,
>> generated file?
>
> We took the old, generated file and removed the platform dependent
> files. They are still implicitly included by the other header files. We
> also added some ifdefs to handle the different build targets.
>
>> - what is the policy of maintaining "src/share/vm/precompiled.hpp",
>> i.e. when should a new include statement be added just into the
>> requiring .cpp file and when will it be appropriate to add it to
>> precompiled.hpp.
>
> Currently, there's no policy for updating this file. We should only
> update this file when trying to optimize the build times. Adding a new
> header to any of the other header files will probably add it to the
> precompiled header. MakeDeps added almost all header files to
> precompiled.hpp, so this is not that different. The precompiled.hpp
> could probably be tweaked for even better build performance than we had
> before this change.
>
>> - did you check the build performance with your changes on the various
>> platforms? Are there any regressions?
>
> We measured the build times on linux and windows and saw no performance
> regressions. Also, remember that we don't use precompiled headers when
> building with the Sun Studio compiler.
>
>> And please, please synchronize this change with other changes,
>> especially the other big ones like Coleens Symtable change to avoid
>> merges as described here:
>>
>> http://old.nabble.com/Request-for-review-%28XL%29-6990754%3A-Use-native-memory-and-reference-counting-to-implement-SymbolTable-tt30138063.html#a30138063
>>
>>
>> Please talk to Coleen who proposed that "..we could arrange that these
>> symbol changes are pushed to hotspot-rt, run through the nightly tests
>> and then pushed to hotspot repository before the other baselines.
>> Maybe it should rate it's own integration slot. I won't create merge
>> change sets before pushing."
>
> Yes, we're currently discussing how this should be pushed.
>
>> Your change patch for example already doesn't run with the current
>> hotspot-rt because change "6970683: improvements to hs_err output"
>> which was already pushed 4 weeks ago added a call to
>> VMError::fatal_error_in_progress() to parallelScavengeHeap.cpp which
>> isn't reflected by your change (i.e. VMError isn't visible). So if you
>> would push your change right now, this would result in a non-empty,
>> potentially quite large merge set. So please apply one of the
>> techniques mentioned in the thread referenced above to avoid such
>> merges.
>
> We will not push the patch as it is. The include statements are
> generated by a tool, which we'll run against the head of hotspot-rt
> before pushing our changes.
>
> Thanks,
> Stefan
>
>> Thank you and best regards,
>> Volker
>>
>> On Wed, Nov 17, 2010 at 4:57 PM, Stefan Karlsson
>> <stefan.karlsson at oracle.com> wrote:
>>> We are simplifying the Hotspot build process by standardizing the
>>> code and
>>> reducing the number of required tools. As a first step we are
>>> removing the
>>> use of the MakeDeps tool and the includeDB files.
>>>
>>> This will help new developers get up to speed with the code, which will
>>> benefit both internal developers and the open source community. It
>>> will also
>>> make IDE integration simpler.
>>>
>>> MakeDeps provided these services:
>>> * Includes between source files
>>> * Platform dispatching
>>> * Precompiled header management
>>> * Makefile rules for source file dependencies
>>> * Source files selection based on compiled target (compiler1, tiered,
>>> kernel, ...)
>>> * VisualStudio project creation
>>> * Circular dependency detection
>>>
>>> To replace MakeDeps we updated all source files with explicit include
>>> statements, include guards and platform dispatching with #ifdefs. We
>>> also
>>> updated the makefiles to provide the rest of the MakeDeps services.
>>>
>>> Note: this change will update almost all source files and many
>>> makefiles.
>>> The top-level make commands have not been changed.
>>>
>>> Webrev is available here:
>>> http://cr.openjdk.java.net/~stefank/6989984.2/
>>>
>>> Bug:
>>> 6989984: Use standard include model for Hotspot
>>> http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=6989984
>>>
>>> Thanks,
>>> Stefan and Bengt
>>>
>


-- 
Dmitry Samersoff
J2SE Sustaining team, SPB04
* Give Rabbit time and he'll always get the answer ...