On Feb 4 2013, at 05:54 , Alan Bateman wrote: > On 02/02/2013 20:40, Mike Duigou wrote: >> I have updated both the jdk repo patch: >> >> 8006594: Add jdk_core target to jdk/test/Makefile >> http://cr.openjdk.java.net/~mduigou/JDK-8006594/1/webrev/ >> >> and the root repo patch: >> >> 8006595: Use jdk/test/Makefile targets in preference to local definitions >> http://cr.openjdk.java.net/~mduigou/JDK-8006595/1/webrev/ >> >> The primary difference between this version and the previous is the >> judicious use of "-k" option and silencing of command spew. A top-level make >> test TEST=jdk_core now succeeds even if sub-tests fail. >> >> I investigated changing the behaviour which determines the test output >> location so that all jdk_core output would be in a single directory. I have >> opted to not change it yet as I can't be certain what depends upon the >> current behaviour. >> >> I would like to change one additional aspect--currently the "prep" target >> forces the output dir to be wiped via a dependency on "clean". I would like >> to remove this dependency and not wipe the output dir with each run. For RE >> and JPRT uses they are likely to be running in fresh directories anyway so >> this change should only impact developers who likely wouldn't want the >> complete wipe anyway. >> >> There are some things about the jtreg_tests target which seem very specific >> to how JPRT uses this makefile. Not all of the behaviours, such as bundling >> of test output into zip files, make sense for all likely users of this >> makefile. I will work with Kelly to partition the JPRT functionality and >> better understand how this Makefile is used by JPRT for future patches. >> >> Mike > If I understand correctly, then the default will now be jdk_core + langtools > but no hotspot (just checking that this is what is intended).
Correct. This is actually unchanged. I am not sure if we should redefine the default to do a broad but shallow smoke test of all repos. I believe this would be the most useful default. The jdk_core set can be expanded/altered if we would like a wider cross section of tests run. > I agree that having JPRT config in the repo is confusing, especially with the > *.test.targets properties that overlap/conflict with the make files. For now I don't want to break/change any existing usages. > > common/makefiles/Main.gmk - it looks like you've added CONCURRENCY=$(JOBS). > Is $(JOBS) coming from the --num-cores specified to configure? Correct. > If so then I think we have to careful because -concurrency means a lot of > virtual memory and I'm not convinced that we limit it via -vmoption in > jdk/test/Makefile > I could see us wanting to dial this down on 32-bit Windows for example. We do limit the max size via -Xmx512M which does seem quite small and is generally well balanced with the number of cores. I opened a build/infrastructure issue to have MEMORY_SIZE and perhaps some suggested VM sizes recorded to the spec.gmk I planned to revisit this again once only new build is supported. > Also I'm not 100% sure that jdk/test/TEST.ROOT is up to date for the client > area -- that's the place where excludeAccess.dirs lists the directories with > tests that cannot run concurrently. The makefile currently segregates tests to use othervm or agentvm by target ie. jdk_beans1 . I haven't checked whether these are synchronized against TEST.ROOT and indeed it would be a shame if these had not been kept in sync. (This is why I added notes about "keep this in sync with ..." in various places) I will check this with Stuart Marks today and plan remediation if necessary. > The webrev has an updated to webrev.ksh, I assume we should ignore that. Correct, spillover from 8004726 patch. (which also needs review. :-) ) > > In jdk/test/Makefile you have silenced the jtreg command but I think that is > very useful to have in the log files. It does produce a lot of spew but I will unsilence it. When the old build is retired we can integrate the new build logging control and make this controllable. > Minor nit in jdk/test/Makefile but the jdk_core recipe could be split over > multiple lines to avoid the very long line. I will do so. > Otherwise I think this looks okay to me. As I mentioned in the original mail > I do think we need to move to the point where the mappings is not in the make > files and hopefully a future installation will get us moving in that > direction. > > -Alan
