Looks good to me.
/Erik
On 2013-02-08 19:30, Mikael Vidstedt wrote:
Please review the following change:
Webrev:
http://cr.openjdk.java.net/~mikael/webrevs/8007639/webrev.00/webrev
Bug: http://bugs.sun.com/view_bug.do?bug_id=8007639
This change corrects the workaround that was introduced in vm.make for
enabling ccache for HotSpot builds. The change introduces a file
specific makefile variable (CFLAGS/<file.o>) which is used to only set
the JRE_RELEASE_VERSION define when compiling vm_version.o.
Verified manually by observing command lines with/without fix, also
passes JPRT.
Some additional background below, more information is available in the
bug report:
To enable the use of ccache 7132779 [1] introduced some new makefile
logic in vm.make to only pass the JRE_RELEASE_VERSION define when
compiling the vm_version.cpp file. The underlying reason for that is
that the JRE_RELEASE_VERSION can in some cases contain a time stamp
and since ccache checks that the defines match before reusing the
cache version of the object file that would mean that if the time
stamp changed all files would have to be recompiled. With the fix only
vm_version.cpp needs to be recompiled.
This almost works, but the logic introduced in the makefile is
actually incorrect. Today it looks like this:
# This is VERY important! The version define must only be supplied to
vm_version.o
# If not, ccache will not re-use the cache at all, since the version
string might contain
# a time and date.
vm_version.o: CXXFLAGS += ${JRE_VERSION}
However, the above syntax does not only add the ${JRE_VERSION} to the
CXXFLAGS of vm_version.o as originally intended - it also /in some
cases/ adds it to the CXXFLAGS for any and all prerequisites of
vm_version.o. And vm_version.o depends on all other object files,
which means in theory this actually passes in that potentially time
stamped version string define to all object files anyway. For various
reasons in reality it only passes it to files that are
lexicographically after vm_version.o - see bug report for more
background on why - but there's still a handful of files that will be
recompiled unnecessarily.
Cheers,
Mikael
[1] http://bugs.sun.com/view_bug.do?bug_id=7132779
