review request (S): 8042255 make gc src file exclusion more automatic

[John Coomes]

David Holmes (david.holmes at oracle.com) wrote:
> Hi John,
> 
> On 14/05/2014 7:34 AM, John Coomes wrote:
> > John Coomes (John.Coomes at oracle.com) wrote:
> >> Please review this small makefile change to reduce the need to update
> >> the list of files in make/excludeSrc.make.
> 
> They will still need to know whether a new file is an excluded file or a 
> kept file, and possibly update the gc_shared_keep list, but at least an 
> excluded but needed file should result in a build error; whereas a 
> not-excluded unneeded file may not.

Hi David,

Thanks for looking at it.

Yes, you're exactly right; I found 4 files in gc_implementation/shared
that were unneeded but not excluded.  At least all cms, g1, parallel
scavenge and parnew files will be excluded automatically.

> Is it worth considering splitting the shared directory in two?

I thought about it, but only for a moment :-).  I think we should see
how often excludeSrc.make has to be updated in jdk9 after this change;
probably won't happen much.

> >>
> >> http://cr.openjdk.java.net/~jcoomes/8042255-gc-exclude/
> >
> > A couple of people on the gc team have looked at these changes and
> > don't object, but they also requested that someone with real makefile
> > expertise take a look.  Anyone here willing to look at it?  Only one
> > file changed.
> 
> This looks okay - assuming the list of excluded files is the same before 
> and after.

There are 4 files that are newly-excluded (see the comments in the
webrev), but they should be excluded.

> These:
> 
>   90       gc_shared_all = $(notdir $(wildcard $(gc_impl)/shared/*.cpp))
>   91       gc_shared_keep =                                        \
> 
> might be slightly more efficient if defined using :=
> 
> For that matter it might be more efficient to change:
> 
>    82       Src_Files_EXCLUDE += 
>        \
>    83         $(notdir $(wildcard $(gc_impl)/concurrentMarkSweep/*.cpp)) 
>       \
>    84         $(notdir $(wildcard $(gc_impl)/g1/*.cpp)) 
>        \
>    85         $(notdir $(wildcard $(gc_impl)/parallelScavenge/*.cpp)) 
>        \
>    86         $(notdir $(wildcard $(gc_impl)/parNew/*.cpp))
> 
> to
> 
>    82       gc_files_exclude := 
>       \
>    83         $(notdir $(wildcard $(gc_impl)/concurrentMarkSweep/*.cpp)) 
>       \
>    84         $(notdir $(wildcard $(gc_impl)/g1/*.cpp)) 
>        \
>    85         $(notdir $(wildcard $(gc_impl)/parallelScavenge/*.cpp)) 
>        \
>    86         $(notdir $(wildcard $(gc_impl)/parNew/*.cpp))
> 
>    87 Src_Files_EXCLUDE += gc_files_exclude

Sure, I like that idea and will change to use "simply-expanded" vars.

-John