RFR (S): 8007639: Workaround for ccache in vm.make is incorrect
Mikael Vidstedt
mikael.vidstedt at oracle.com
Wed Feb 13 02:32:37 UTC 2013
Good catch! There is no fundamental reason for switching to CFLAGS, and
since the other defines are added to CXXFLAGS so should this. Updated
webrev here:
http://cr.openjdk.java.net/~mikael/webrevs/8007639/webrev.01/webrev/
Cheers,
Mikael
On 2013-02-12 18:02, David Holmes wrote:
> Oops! Hit reply instead of reply-all. Adding back the lists.
>
> On 12/02/2013 11:36 AM, David Holmes wrote:
>> Hi Mikael,
>>
>> So after our side-bar discussions on other ways to tackle this, the only
>> query I have now is why the original change adjusted CXXFLAGS and this
>> change adjusts CFLAGS instead?
>>
>> Thanks,
>> David
>>
>> On 9/02/2013 4:30 AM, 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
>>>
More information about the build-dev
mailing list