Re: RFR [9] Modular Source Code
On 2014-08-28 21:36, Phil Race wrote: >./windows/native/libawt/sun/java2d/d3d/D3DShaderGen.c Is a tool that is run manually when we need to re-generate the shaders. It is co-located so that can be found easily. It certainly should not be deleted, nor should it be moved anywhere obscure. Oh, I see. There was actually some very good documention in the file itself. :) As a general rule, I think the build system should build all tools that is actively used, even if not so frequent, to avoid bit rot. In this case, we could for instance let configure detect the presence of the DirectX SDK, and if present, compile and run the D3DShaderGen tool. This will increase the likelyhood that the tool is actually working when needed, and that the generated header file is kept up to date. So, yes, decisions on most of these need to be jointly discussed and reviewed although some may be better 'owned' by 2d and some by build. I moved all the bugs but one (that is pure makefile logic) to client-libs/2d. I hope that is the correct sub-component. /Magnus
Re: RFR [9] Modular Source Code
On 28/08/2014 20:36, Phil Race wrote: * All of the awt-related directories (libawt_* and common) include an unnecessary extra layer, the "sun" directory. It is not needed anymore, Let's *not* do that. It also affects the destination package. Remember sun.* is the protected top-level package .. and people won't always be running in jigsaw mode. For native/libXXX then there shouldn't be any need to mirror the java package structure in the native tree now. The shuffle should have done this for most libraries, libawt may be the exception and it would be good to get it consistent if possible. There isn't a "jigsaw mode", and nothing that I can think of that would have an impact on the source structure. Plus I recently learned that compact profiles need to be informed when you do something like this. The profiles build shouldn't be concerned with the source layout as it's the equivalent of an images build. So for the native code then it just needs to know which already-built native libraries to include. I suspect your comment may be related to the refactor of the datatransfer API, in which case it was a java package rather than a native libraries that just a temporary break (no big deal, easily fixed). In time we will replace the profiles build, probably when we move to the modular image as each of the images (JDK, JRE, compact1, compact2, ...) will be just built from a set of modules. -Alan.
Re: RFR [9] Modular Source Code
* All of the awt-related directories (libawt_* and common) include an unnecessary extra layer, the "sun" directory. It is not needed anymore, Let's *not* do that. It also affects the destination package. Remember sun.* is the protected top-level package .. and people won't always be running in jigsaw mode. Plus I recently learned that compact profiles need to be informed when you do something like this. >./windows/native/libawt/sun/java2d/d3d/D3DShaderGen.c Is a tool that is run manually when we need to re-generate the shaders. It is co-located so that can be found easily. It certainly should not be deleted, nor should it be moved anywhere obscure. makecube is similar but is I think on a list to consider deletion since I don't think we need it anymore. But launchers would be the wrong place. So, yes, decisions on most of these need to be jointly discussed and reviewed although some may be better 'owned' by 2d and some by build. -phil. On 8/28/2014 11:38 AM, Anthony Petrov wrote: Thanks! -- best regards, Anthony On 8/28/2014 1:00 PM, Magnus Ihse Bursie wrote: On 2014-08-27 12:57, Anthony Petrov wrote: Hi Magnus, Those look like reasonable suggestions. Could you please file separate bugs for each of these issues? Also, please note that most of them belong to 2D, not AWT (e.g. the font-, color-, d3d-, and opengl-related files) even though they're compiled into the awt native libraries. I created JDK-8056209, JDK-8056210, JDK-8056212, JDK-8056213, JDK-8056214, JDK-8056215, JDK-8056216 and JDK-8056217 for these. After some consideration, I choose to put them on the infrastructure/build category, since I believe the build part (changes to the makefiles) requires most of the work (compared to e.g. removing a file), and that we are probably more keen on having them solved :), but they need to be resolved in cooperation with the 2D team. The issue regarding the medialib directories was already reported in JDK-8055190. /Magnus -- best regards, Anthony On 8/25/2014 1:14 PM, Magnus Ihse Bursie wrote: On 2014-08-20 11:14, Magnus Ihse Bursie wrote: On 2014-08-18 16:15, Anthony Petrov wrote: So I'm not sure if the current set of AWT libraries could be simplified any further. Hope this helps. Thank you for the clarification, it was very helpful! While the set of AWT libraries probably cannot be simplified as you say, my gut feeling is still that the current layout of files on disk does not optimally match the actual libraries we build. Armed with the help of your description, I'll look into them once again and see if I can make that statement more concrete. This is my suggestions based on what I found when trying to remove the last unnecessary entanglement. Note that all paths are relative to the java.desktop module, and that I have at this stage only looked at compiled sources (*.c), not header files. * The following files are in the windows directory tree, but are explicitly excluded on Windows. Thus they will never be built, and should be removed instead: ./windows/native/libawt/sun/java2d/d3d/D3DPipeline.cpp ./windows/native/libawt/sun/java2d/d3d/D3DShaderGen.c ./windows/native/libawt/sun/windows/WBufferStrategy.cpp * The following file is in the share directory tree, but is only used on Windows. It should be moved to the corresponding windows directory instead: ./share/native/libawt/sun/java2d/ShaderList.c * The following directory is in the unix directory tree, but is only used on Solaris. It should be moved to the corresponding solaris directory instead: ./unix/native/libawt/sun/java2d/loops * The directory ./unix/native/common/sun/awt contains five more or less unrelated .c files. Three of them are only used in libawt_xawt, and should be moved there: ./unix/native/common/sun/awt/awt_Font.c ./unix/native/common/sun/awt/fontpath.c ./unix/native/common/sun/awt/X11Color.c Of the remaining two CUPSfuncs.c seems correctly placed, since it is shared between libawt_xawt and libawt_lwawt. However, I'm wondering about initIDs.c. It is compiled in libawt as well as libawt_xawt, but when I checked some random functions, they are exported (via the mapfile) for libawt only. So I believe it is a mistake to include it in libawt_xawt, and that it should be moved to the libawt directory. This will need verification from someone on the AWT team. * The directory ./unix/native/libjawt is included in libawt_xawt (and in libjawt, of course). This seems suspicious to me. There is just a single file with a single function, JAWT_GetAWT(), which is exported in libjawt (via a mapfile), but not in libawt_xawt. I believe it is a mistake to include it in libawt_xawt. This will need verification from someone on the AWT team. * All of the awt-related directories (libawt_* and common) include an unnecessary extra layer, the "sun" directory. It is not needed anymore, and just makes all paths extra long. I suggest that we remove that layer and move everything up on
Re: RFR [9] Modular Source Code
Thanks! -- best regards, Anthony On 8/28/2014 1:00 PM, Magnus Ihse Bursie wrote: On 2014-08-27 12:57, Anthony Petrov wrote: Hi Magnus, Those look like reasonable suggestions. Could you please file separate bugs for each of these issues? Also, please note that most of them belong to 2D, not AWT (e.g. the font-, color-, d3d-, and opengl-related files) even though they're compiled into the awt native libraries. I created JDK-8056209, JDK-8056210, JDK-8056212, JDK-8056213, JDK-8056214, JDK-8056215, JDK-8056216 and JDK-8056217 for these. After some consideration, I choose to put them on the infrastructure/build category, since I believe the build part (changes to the makefiles) requires most of the work (compared to e.g. removing a file), and that we are probably more keen on having them solved :), but they need to be resolved in cooperation with the 2D team. The issue regarding the medialib directories was already reported in JDK-8055190. /Magnus -- best regards, Anthony On 8/25/2014 1:14 PM, Magnus Ihse Bursie wrote: On 2014-08-20 11:14, Magnus Ihse Bursie wrote: On 2014-08-18 16:15, Anthony Petrov wrote: So I'm not sure if the current set of AWT libraries could be simplified any further. Hope this helps. Thank you for the clarification, it was very helpful! While the set of AWT libraries probably cannot be simplified as you say, my gut feeling is still that the current layout of files on disk does not optimally match the actual libraries we build. Armed with the help of your description, I'll look into them once again and see if I can make that statement more concrete. This is my suggestions based on what I found when trying to remove the last unnecessary entanglement. Note that all paths are relative to the java.desktop module, and that I have at this stage only looked at compiled sources (*.c), not header files. * The following files are in the windows directory tree, but are explicitly excluded on Windows. Thus they will never be built, and should be removed instead: ./windows/native/libawt/sun/java2d/d3d/D3DPipeline.cpp ./windows/native/libawt/sun/java2d/d3d/D3DShaderGen.c ./windows/native/libawt/sun/windows/WBufferStrategy.cpp * The following file is in the share directory tree, but is only used on Windows. It should be moved to the corresponding windows directory instead: ./share/native/libawt/sun/java2d/ShaderList.c * The following directory is in the unix directory tree, but is only used on Solaris. It should be moved to the corresponding solaris directory instead: ./unix/native/libawt/sun/java2d/loops * The directory ./unix/native/common/sun/awt contains five more or less unrelated .c files. Three of them are only used in libawt_xawt, and should be moved there: ./unix/native/common/sun/awt/awt_Font.c ./unix/native/common/sun/awt/fontpath.c ./unix/native/common/sun/awt/X11Color.c Of the remaining two CUPSfuncs.c seems correctly placed, since it is shared between libawt_xawt and libawt_lwawt. However, I'm wondering about initIDs.c. It is compiled in libawt as well as libawt_xawt, but when I checked some random functions, they are exported (via the mapfile) for libawt only. So I believe it is a mistake to include it in libawt_xawt, and that it should be moved to the libawt directory. This will need verification from someone on the AWT team. * The directory ./unix/native/libjawt is included in libawt_xawt (and in libjawt, of course). This seems suspicious to me. There is just a single file with a single function, JAWT_GetAWT(), which is exported in libjawt (via a mapfile), but not in libawt_xawt. I believe it is a mistake to include it in libawt_xawt. This will need verification from someone on the AWT team. * All of the awt-related directories (libawt_* and common) include an unnecessary extra layer, the "sun" directory. It is not needed anymore, and just makes all paths extra long. I suggest that we remove that layer and move everything up one step. * The makefiles include too specific directories. Instead of including e.g. ./*/native/common/sun/java2d/opengl and ./*/native/common/sun/java2d/x11, we should just include ./*/native/common/sun/java2d. This level corresponds to a logical grouping of the source code, and not the directory structure in that grouping. * The file ./windows/native/common/awt_makecube.cpp is a bit strange. It is not a shared file; instead it's a stand-alone binary with a main() function. It is not compiled by any makefile targets. If this file is actually used, I suggest moving it to a better location (windows/native/launchers?), and starting to compile it with the build. (Stuff that's not built regularly is doomed to bit rot.) It if is not used, I suggest we remove it. * And as I stated before, the medlialib directories are typically not used by libawt and friends. It is used by libmlib_image and libmlib_image_v, and should move away from the awt directory. /Magnus
Re: RFR [9] Modular Source Code
On 2014-08-27 12:57, Anthony Petrov wrote: Hi Magnus, Those look like reasonable suggestions. Could you please file separate bugs for each of these issues? Also, please note that most of them belong to 2D, not AWT (e.g. the font-, color-, d3d-, and opengl-related files) even though they're compiled into the awt native libraries. I created JDK-8056209, JDK-8056210, JDK-8056212, JDK-8056213, JDK-8056214, JDK-8056215, JDK-8056216 and JDK-8056217 for these. After some consideration, I choose to put them on the infrastructure/build category, since I believe the build part (changes to the makefiles) requires most of the work (compared to e.g. removing a file), and that we are probably more keen on having them solved :), but they need to be resolved in cooperation with the 2D team. The issue regarding the medialib directories was already reported in JDK-8055190. /Magnus -- best regards, Anthony On 8/25/2014 1:14 PM, Magnus Ihse Bursie wrote: On 2014-08-20 11:14, Magnus Ihse Bursie wrote: On 2014-08-18 16:15, Anthony Petrov wrote: So I'm not sure if the current set of AWT libraries could be simplified any further. Hope this helps. Thank you for the clarification, it was very helpful! While the set of AWT libraries probably cannot be simplified as you say, my gut feeling is still that the current layout of files on disk does not optimally match the actual libraries we build. Armed with the help of your description, I'll look into them once again and see if I can make that statement more concrete. This is my suggestions based on what I found when trying to remove the last unnecessary entanglement. Note that all paths are relative to the java.desktop module, and that I have at this stage only looked at compiled sources (*.c), not header files. * The following files are in the windows directory tree, but are explicitly excluded on Windows. Thus they will never be built, and should be removed instead: ./windows/native/libawt/sun/java2d/d3d/D3DPipeline.cpp ./windows/native/libawt/sun/java2d/d3d/D3DShaderGen.c ./windows/native/libawt/sun/windows/WBufferStrategy.cpp * The following file is in the share directory tree, but is only used on Windows. It should be moved to the corresponding windows directory instead: ./share/native/libawt/sun/java2d/ShaderList.c * The following directory is in the unix directory tree, but is only used on Solaris. It should be moved to the corresponding solaris directory instead: ./unix/native/libawt/sun/java2d/loops * The directory ./unix/native/common/sun/awt contains five more or less unrelated .c files. Three of them are only used in libawt_xawt, and should be moved there: ./unix/native/common/sun/awt/awt_Font.c ./unix/native/common/sun/awt/fontpath.c ./unix/native/common/sun/awt/X11Color.c Of the remaining two CUPSfuncs.c seems correctly placed, since it is shared between libawt_xawt and libawt_lwawt. However, I'm wondering about initIDs.c. It is compiled in libawt as well as libawt_xawt, but when I checked some random functions, they are exported (via the mapfile) for libawt only. So I believe it is a mistake to include it in libawt_xawt, and that it should be moved to the libawt directory. This will need verification from someone on the AWT team. * The directory ./unix/native/libjawt is included in libawt_xawt (and in libjawt, of course). This seems suspicious to me. There is just a single file with a single function, JAWT_GetAWT(), which is exported in libjawt (via a mapfile), but not in libawt_xawt. I believe it is a mistake to include it in libawt_xawt. This will need verification from someone on the AWT team. * All of the awt-related directories (libawt_* and common) include an unnecessary extra layer, the "sun" directory. It is not needed anymore, and just makes all paths extra long. I suggest that we remove that layer and move everything up one step. * The makefiles include too specific directories. Instead of including e.g. ./*/native/common/sun/java2d/opengl and ./*/native/common/sun/java2d/x11, we should just include ./*/native/common/sun/java2d. This level corresponds to a logical grouping of the source code, and not the directory structure in that grouping. * The file ./windows/native/common/awt_makecube.cpp is a bit strange. It is not a shared file; instead it's a stand-alone binary with a main() function. It is not compiled by any makefile targets. If this file is actually used, I suggest moving it to a better location (windows/native/launchers?), and starting to compile it with the build. (Stuff that's not built regularly is doomed to bit rot.) It if is not used, I suggest we remove it. * And as I stated before, the medlialib directories are typically not used by libawt and friends. It is used by libmlib_image and libmlib_image_v, and should move away from the awt directory. /Magnus
Re: RFR [9] Modular Source Code
On 27/08/2014 11:57, Anthony Petrov wrote: Hi Magnus, Those look like reasonable suggestions. Could you please file separate bugs for each of these issues? Also, please note that most of them belong to 2D, not AWT (e.g. the font-, color-, d3d-, and opengl-related files) even though they're compiled into the awt native libraries. -- best regards, Anthony Just to add to Magnus' list, there are a few additional AWT and libmlib_image source files listed in JDK-8047931 that should be examined too. -Alan.
Re: RFR [9] Modular Source Code
Hi Magnus, Those look like reasonable suggestions. Could you please file separate bugs for each of these issues? Also, please note that most of them belong to 2D, not AWT (e.g. the font-, color-, d3d-, and opengl-related files) even though they're compiled into the awt native libraries. -- best regards, Anthony On 8/25/2014 1:14 PM, Magnus Ihse Bursie wrote: On 2014-08-20 11:14, Magnus Ihse Bursie wrote: On 2014-08-18 16:15, Anthony Petrov wrote: So I'm not sure if the current set of AWT libraries could be simplified any further. Hope this helps. Thank you for the clarification, it was very helpful! While the set of AWT libraries probably cannot be simplified as you say, my gut feeling is still that the current layout of files on disk does not optimally match the actual libraries we build. Armed with the help of your description, I'll look into them once again and see if I can make that statement more concrete. This is my suggestions based on what I found when trying to remove the last unnecessary entanglement. Note that all paths are relative to the java.desktop module, and that I have at this stage only looked at compiled sources (*.c), not header files. * The following files are in the windows directory tree, but are explicitly excluded on Windows. Thus they will never be built, and should be removed instead: ./windows/native/libawt/sun/java2d/d3d/D3DPipeline.cpp ./windows/native/libawt/sun/java2d/d3d/D3DShaderGen.c ./windows/native/libawt/sun/windows/WBufferStrategy.cpp * The following file is in the share directory tree, but is only used on Windows. It should be moved to the corresponding windows directory instead: ./share/native/libawt/sun/java2d/ShaderList.c * The following directory is in the unix directory tree, but is only used on Solaris. It should be moved to the corresponding solaris directory instead: ./unix/native/libawt/sun/java2d/loops * The directory ./unix/native/common/sun/awt contains five more or less unrelated .c files. Three of them are only used in libawt_xawt, and should be moved there: ./unix/native/common/sun/awt/awt_Font.c ./unix/native/common/sun/awt/fontpath.c ./unix/native/common/sun/awt/X11Color.c Of the remaining two CUPSfuncs.c seems correctly placed, since it is shared between libawt_xawt and libawt_lwawt. However, I'm wondering about initIDs.c. It is compiled in libawt as well as libawt_xawt, but when I checked some random functions, they are exported (via the mapfile) for libawt only. So I believe it is a mistake to include it in libawt_xawt, and that it should be moved to the libawt directory. This will need verification from someone on the AWT team. * The directory ./unix/native/libjawt is included in libawt_xawt (and in libjawt, of course). This seems suspicious to me. There is just a single file with a single function, JAWT_GetAWT(), which is exported in libjawt (via a mapfile), but not in libawt_xawt. I believe it is a mistake to include it in libawt_xawt. This will need verification from someone on the AWT team. * All of the awt-related directories (libawt_* and common) include an unnecessary extra layer, the "sun" directory. It is not needed anymore, and just makes all paths extra long. I suggest that we remove that layer and move everything up one step. * The makefiles include too specific directories. Instead of including e.g. ./*/native/common/sun/java2d/opengl and ./*/native/common/sun/java2d/x11, we should just include ./*/native/common/sun/java2d. This level corresponds to a logical grouping of the source code, and not the directory structure in that grouping. * The file ./windows/native/common/awt_makecube.cpp is a bit strange. It is not a shared file; instead it's a stand-alone binary with a main() function. It is not compiled by any makefile targets. If this file is actually used, I suggest moving it to a better location (windows/native/launchers?), and starting to compile it with the build. (Stuff that's not built regularly is doomed to bit rot.) It if is not used, I suggest we remove it. * And as I stated before, the medlialib directories are typically not used by libawt and friends. It is used by libmlib_image and libmlib_image_v, and should move away from the awt directory. /Magnus
Re: RFR [9] Modular Source Code
On 2014-08-20 11:14, Magnus Ihse Bursie wrote: On 2014-08-18 16:15, Anthony Petrov wrote: So I'm not sure if the current set of AWT libraries could be simplified any further. Hope this helps. Thank you for the clarification, it was very helpful! While the set of AWT libraries probably cannot be simplified as you say, my gut feeling is still that the current layout of files on disk does not optimally match the actual libraries we build. Armed with the help of your description, I'll look into them once again and see if I can make that statement more concrete. This is my suggestions based on what I found when trying to remove the last unnecessary entanglement. Note that all paths are relative to the java.desktop module, and that I have at this stage only looked at compiled sources (*.c), not header files. * The following files are in the windows directory tree, but are explicitly excluded on Windows. Thus they will never be built, and should be removed instead: ./windows/native/libawt/sun/java2d/d3d/D3DPipeline.cpp ./windows/native/libawt/sun/java2d/d3d/D3DShaderGen.c ./windows/native/libawt/sun/windows/WBufferStrategy.cpp * The following file is in the share directory tree, but is only used on Windows. It should be moved to the corresponding windows directory instead: ./share/native/libawt/sun/java2d/ShaderList.c * The following directory is in the unix directory tree, but is only used on Solaris. It should be moved to the corresponding solaris directory instead: ./unix/native/libawt/sun/java2d/loops * The directory ./unix/native/common/sun/awt contains five more or less unrelated .c files. Three of them are only used in libawt_xawt, and should be moved there: ./unix/native/common/sun/awt/awt_Font.c ./unix/native/common/sun/awt/fontpath.c ./unix/native/common/sun/awt/X11Color.c Of the remaining two CUPSfuncs.c seems correctly placed, since it is shared between libawt_xawt and libawt_lwawt. However, I'm wondering about initIDs.c. It is compiled in libawt as well as libawt_xawt, but when I checked some random functions, they are exported (via the mapfile) for libawt only. So I believe it is a mistake to include it in libawt_xawt, and that it should be moved to the libawt directory. This will need verification from someone on the AWT team. * The directory ./unix/native/libjawt is included in libawt_xawt (and in libjawt, of course). This seems suspicious to me. There is just a single file with a single function, JAWT_GetAWT(), which is exported in libjawt (via a mapfile), but not in libawt_xawt. I believe it is a mistake to include it in libawt_xawt. This will need verification from someone on the AWT team. * All of the awt-related directories (libawt_* and common) include an unnecessary extra layer, the "sun" directory. It is not needed anymore, and just makes all paths extra long. I suggest that we remove that layer and move everything up one step. * The makefiles include too specific directories. Instead of including e.g. ./*/native/common/sun/java2d/opengl and ./*/native/common/sun/java2d/x11, we should just include ./*/native/common/sun/java2d. This level corresponds to a logical grouping of the source code, and not the directory structure in that grouping. * The file ./windows/native/common/awt_makecube.cpp is a bit strange. It is not a shared file; instead it's a stand-alone binary with a main() function. It is not compiled by any makefile targets. If this file is actually used, I suggest moving it to a better location (windows/native/launchers?), and starting to compile it with the build. (Stuff that's not built regularly is doomed to bit rot.) It if is not used, I suggest we remove it. * And as I stated before, the medlialib directories are typically not used by libawt and friends. It is used by libmlib_image and libmlib_image_v, and should move away from the awt directory. /Magnus
Re: RFR [9] Modular Source Code
On 18/08/2014 14:56, Magnus Ihse Bursie wrote: : * In CompileDemos, we create demo/jpda/src.zip. This includes source code from src/demo/share (expected), but also from jdk/src/jdk.jdi/share/classes/com/sun/tools/example/... (unexpected!). Should example code really live in the jdk.jdi package, and not in src/demo? The issue is that the sources to jdb are used for two purposes. This should resolve itself once the JPDA demo is dropped, see JDK-8043981. -Alan.
Re: RFR [9] Modular Source Code
On 2014-08-15 10:52, Magnus Ihse Bursie wrote: These are indeed the single most significant changes to the build system since the "new" build system was introduced. Here follows a partial review of the changes in the jdk repo. Even though it's long, I'm not done. ;-) While this has turned into a post-factum code review, I continue unabashed. :-) In langtooks/make/GensrcLangtools.gmk: The fix from 8026773 was inadvertently reverted in the following line: $(ECHO) Compiling $(words $(PROPSOURCES) v1 v2 v3) properties into resource bundles This might implicate a merge error at some point, so maybe the file should be more thoroughly studied if there are more parts of the fix for 8026773 that has been lost. In make/common/JavaCompilation: The following stanza is incorrectly indented: ifneq (,$$($1_ALL_COPIES)) # Yep, there are files to be copied! $1_ALL_COPY_TARGETS:= $$(foreach i,$$($1_ALL_COPIES),$$(eval $$(call add_file_to_copy,$1,$$i))) # Now we can depend on $$($1_ALL_COPY_TARGETS) to copy all files! endif In make/CompileJavaModules: A typo: ## Service types are required in the classpath when compiing module-info In make/common/support: The ListPathsSafely support files have changed, replacing the first three strings with: s|X01|share/classes|g s|X02|internal|g s|X03|com/sun/org|g While the old replacements did not do anything useful ("shortening" three-letter strings into another three-letter string), this change breaks the old pattern of having the strings sorted in length order. At first, I thought it was just a matter of style (I still think this is a matter of style, and that it should be fixed) but then I realized there also was a reason for this: The strings are matched from longest to shortest (given, of course, that the list is correctly ordered) and thus the string "com/sun" will match and be replaced by X07 before the new string "com/sun/org" will be given the chance. In corba/make and langtools/make: For clarity and consistency, the file corba/make/CompileCorba.gmk should be renamed CompileInterimCorba.gmk and langtools/make/CompileInterim.gmk should be renamed CompileInterimLangtools.gmk. In make/Main.gmk: I'm not entirely happy with the way the following dependencies are encoded: # Declare dependencies from all other -lib to java.base-lib $(foreach t, $(filter-out java.base-libs, $(LIB_TARGETS)), \ $(eval $t: java.base-libs)) # Declare the special case dependency for jdk.deploy.osx where libosx # links against libosxapp. jdk.deploy.osx-libs: java.desktop-libs # This dependency needs to be explicitly declared. jdk.jdi-gensrc generates a # header file used by jdk.jdwp libs. jdk.jdwp-libs: jdk.jdi-gensrc I have discussed this off-line with Erik and concluded that this probably has to do for now, but I believe there should be a better way of describing this kinds of relationships. Alright, now I'm done code reviewing! :-) Lots of kudos to Erik for managing to re-create the build system for the modular source code with such high quality. /Magnus
Re: RFR [9] Modular Source Code
On 2014-08-18 16:15, Anthony Petrov wrote: On 8/18/2014 5:47 PM, Magnus Ihse Bursie wrote: libawt et al: The relation between libawt, libawt_headless, libjawt, libawt_xawt and libawt_lwawt are hairy enough to make my brain curl up. I believe there are simplifications to be made but I gave up trying to figure them out. libawt contains code that is shared between all AWT lib implementations. Depending on what mode/toolkit is requested, it loads a specific variant of the awt native library, such as: - libawt_headless - headless AWT implementation. - libawt_xawt - XToolkit implementation (uses X11 for GUI) - libawt_lwawt - CToolkit implementation (uses Cocoa for GUI) Historically, we were able to choose between lwawt and xawt on Mac in the past, but we no longer support or even build xawt on Mac. Also, in the past there was MToolkit (which used Motif for GUI). Again, we no longer support this toolkit. So, currently we always use xawt on Linux/Solaris and lwawt on Mac. But since we still need to choose between a real toolkit and a headless toolkit, we need the common libawt library. libjawt is a helper library that implements JAWT API and is loaded by applications that use the JAWT interface which allows them to get direct access to the native AWT drawing surface and use native APIs (e.g. OpenGL) to draw on the surface. This library isn't needed for regular AWT/Swing applications. So I'm not sure if the current set of AWT libraries could be simplified any further. Hope this helps. Thank you for the clarification, it was very helpful! While the set of AWT libraries probably cannot be simplified as you say, my gut feeling is still that the current layout of files on disk does not optimally match the actual libraries we build. Armed with the help of your description, I'll look into them once again and see if I can make that statement more concrete. /Magnus
Re: RFR [9] Modular Source Code
Hello, The basic rule for the new source layout is that for each library, there is a directory where all sources for that library go. This was hard to apply to libawt and friends since as you say, some files go in libawt on windows and libawt_xawt on linux and solaris. For now, I put those in common for lack of a better place. /Erik On 2014-08-20 01:09, Phil Race wrote: On a related note I am scratching my head about why some files destined to be compiled into libawt or libawt_xawt go into a directory called 'common'. Eg OpenGL sources are in common but aren't common to all libs or even to all awt libs, since they would go into libawt on windows and libawt_xawt on unix and not at all into libawt_headless. 'common 'might (guessing here) have started out as place to put interface header files shared between libs but maybe got adopted for other uses ? -phil. On 08/18/2014 07:15 AM, Anthony Petrov wrote: On 8/18/2014 5:47 PM, Magnus Ihse Bursie wrote: libawt et al: The relation between libawt, libawt_headless, libjawt, libawt_xawt and libawt_lwawt are hairy enough to make my brain curl up. I believe there are simplifications to be made but I gave up trying to figure them out. libawt contains code that is shared between all AWT lib implementations. Depending on what mode/toolkit is requested, it loads a specific variant of the awt native library, such as: - libawt_headless - headless AWT implementation. - libawt_xawt - XToolkit implementation (uses X11 for GUI) - libawt_lwawt - CToolkit implementation (uses Cocoa for GUI) Historically, we were able to choose between lwawt and xawt on Mac in the past, but we no longer support or even build xawt on Mac. Also, in the past there was MToolkit (which used Motif for GUI). Again, we no longer support this toolkit. So, currently we always use xawt on Linux/Solaris and lwawt on Mac. But since we still need to choose between a real toolkit and a headless toolkit, we need the common libawt library. libjawt is a helper library that implements JAWT API and is loaded by applications that use the JAWT interface which allows them to get direct access to the native AWT drawing surface and use native APIs (e.g. OpenGL) to draw on the surface. This library isn't needed for regular AWT/Swing applications. So I'm not sure if the current set of AWT libraries could be simplified any further. Hope this helps. -- best regards, Anthony
Re: RFR [9] Modular Source Code
On a related note I am scratching my head about why some files destined to be compiled into libawt or libawt_xawt go into a directory called 'common'. Eg OpenGL sources are in common but aren't common to all libs or even to all awt libs, since they would go into libawt on windows and libawt_xawt on unix and not at all into libawt_headless. 'common 'might (guessing here) have started out as place to put interface header files shared between libs but maybe got adopted for other uses ? -phil. On 08/18/2014 07:15 AM, Anthony Petrov wrote: On 8/18/2014 5:47 PM, Magnus Ihse Bursie wrote: libawt et al: The relation between libawt, libawt_headless, libjawt, libawt_xawt and libawt_lwawt are hairy enough to make my brain curl up. I believe there are simplifications to be made but I gave up trying to figure them out. libawt contains code that is shared between all AWT lib implementations. Depending on what mode/toolkit is requested, it loads a specific variant of the awt native library, such as: - libawt_headless - headless AWT implementation. - libawt_xawt - XToolkit implementation (uses X11 for GUI) - libawt_lwawt - CToolkit implementation (uses Cocoa for GUI) Historically, we were able to choose between lwawt and xawt on Mac in the past, but we no longer support or even build xawt on Mac. Also, in the past there was MToolkit (which used Motif for GUI). Again, we no longer support this toolkit. So, currently we always use xawt on Linux/Solaris and lwawt on Mac. But since we still need to choose between a real toolkit and a headless toolkit, we need the common libawt library. libjawt is a helper library that implements JAWT API and is loaded by applications that use the JAWT interface which allows them to get direct access to the native AWT drawing surface and use native APIs (e.g. OpenGL) to draw on the surface. This library isn't needed for regular AWT/Swing applications. So I'm not sure if the current set of AWT libraries could be simplified any further. Hope this helps. -- best regards, Anthony
Re: RFR [9] Modular Source Code
* Magnus Ihse Bursie [2014-08-18 09:48]: > liblcms: > The include file picking of LCMS.c would not be needed if the upstream > lcms source code were moved into a separate subdirectory of liblcms. > > libjavajpeg: > If the upstream IJG jpeg library were moved into a subdirectory, the > explicit includes of imageIOJPEG.c and jpegdecoder.c would not be needed. Independent of the modular source code changes, I would like to see this change. But there were concerns raised in the past about this: http://mail.openjdk.java.net/pipermail/build-dev/2014-February/012027.html Maybe now is the right time to clean this up, once and for all? Thanks, Omair -- PGP Key: 66484681 (http://pgp.mit.edu/) Fingerprint = F072 555B 0A17 3957 4E95 0056 F286 F14F 6648 4681
Re: RFR [9] Modular Source Code
On 16/08/2014 1:58 AM, Erik Joelsson wrote: Hello Omair, No, as I tried to explain in my initial mail on this thread, since the modules are all compiled in parallel, and not sequentially like the current build does with the repositories, it doesn't make much sense timing each module. The numbers would be meaningless. It would still be nice to see the different times for hotspot, images, any other targets, plus the "everything Java" component. Seeing where our build time gets used up is important to some of us. Thanks, David - /Erik On 2014-08-15 17:55, Omair Majid wrote: Hi, * Alan Bateman [2014-08-13 08:36]: Just to add to Chris and Erik's mails, I would encourage everyone that pushes to jdk9/dev or the other jdk9 project integration forests to clone and build jigsaw/stage and get familiar with the proposed layout, new build targets, and the very different output emitted during the build. The changes are arguably as significant as the transition in JDK 8 from the "old build" to the "new build" so the more people taking the forest for a test drive the better. If you maintain your our own own IDE project then you'll likely have to adjust file paths so any issues encountered would be useful to hear about too. Just one RFE for now: The new build would provide a detailed breakdown of the build time of each repo. With the 'new new build', all I see is an overall time: - Build times --- Start 2014-08-15 10:59:27 End 2014-08-15 11:17:53 00:18:26 TOTAL - Will a detailed breakdown of the build times for each module make a return? Thanks, Omair
Re: RFR [9] Modular Source Code
On 8/18/2014 5:47 PM, Magnus Ihse Bursie wrote: libawt et al: The relation between libawt, libawt_headless, libjawt, libawt_xawt and libawt_lwawt are hairy enough to make my brain curl up. I believe there are simplifications to be made but I gave up trying to figure them out. libawt contains code that is shared between all AWT lib implementations. Depending on what mode/toolkit is requested, it loads a specific variant of the awt native library, such as: - libawt_headless - headless AWT implementation. - libawt_xawt - XToolkit implementation (uses X11 for GUI) - libawt_lwawt - CToolkit implementation (uses Cocoa for GUI) Historically, we were able to choose between lwawt and xawt on Mac in the past, but we no longer support or even build xawt on Mac. Also, in the past there was MToolkit (which used Motif for GUI). Again, we no longer support this toolkit. So, currently we always use xawt on Linux/Solaris and lwawt on Mac. But since we still need to choose between a real toolkit and a headless toolkit, we need the common libawt library. libjawt is a helper library that implements JAWT API and is loaded by applications that use the JAWT interface which allows them to get direct access to the native AWT drawing surface and use native APIs (e.g. OpenGL) to draw on the surface. This library isn't needed for regular AWT/Swing applications. So I'm not sure if the current set of AWT libraries could be simplified any further. Hope this helps. -- best regards, Anthony
Re: RFR [9] Modular Source Code
I also found these random notes scribbled down that I forgot to add to the previous mails: * In CoreLibraries.gmk, we have dead code defining BUILD_LIBVERIFY_SRC which is not used anymore. * In Tools.gmk, I notice that we copy the files $(JDK_TOPDIR)/src/java.desktop/share/classes/javax/swing/plaf/nimbus/%.template, but if they are only used by our build tools, they should probably live in make/data somewhere. * In SoundLibraries.gmk, the source code splitting is not complete. The directory libjsound is used to build not only libjsound but libjsoundalsa and libjsoundds, and thus needs a complex include/exclude system like before. * In CompileDemos, we create demo/jpda/src.zip. This includes source code from src/demo/share (expected), but also from jdk/src/jdk.jdi/share/classes/com/sun/tools/example/... (unexpected!). Should example code really live in the jdk.jdi package, and not in src/demo? /Magnus
Re: RFR [9] Modular Source Code
On 2014-08-15 12:13, Magnus Ihse Bursie wrote: Here are the more concrete specification of this, for all files except Awt2dLibraries.gmk, which I'll return to. And here is the jury's verdict on Awt2dLibraries.gmk. :-) libjavajpeg: One of these are not needed: LIBJAVAJPEG_SRC := $(JDK_TOPDIR)/src/java.desktop/share/native/libjavajpeg LIBJAVAJPEG_SRC += $(JDK_TOPDIR)/src/java.desktop/share/native/libjavajpeg libfontmanager: EXCLUDE_FILES := $(LIBFONTMANAGER_EXCLUDE_FILES) \ AccelGlyphCache.c, \ The file AccelGlyphCache.c resides in src/java.desktop/share/native/common/sun/font, which is not included in the source dirs, so the exclude can be safely removed. The following stanza can be removed: ifeq ($(OPENJDK_TARGET_OS), windows) LIBFONTMANAGER_EXCLUDE_FILES += X11FontScaler.c \ X11TextRenderer.c LIBFONTMANAGER_OPTIMIZATION := HIGHEST LIBFONTMANAGER_CFLAGS += -I$(JDK_TOPDIR)/src/java.desktop/$(OPENJDK_TARGET_OS_API_DIR)/native/libawt/sun/windows else ifeq ($(OPENJDK_TARGET_OS), macosx) LIBFONTMANAGER_EXCLUDE_FILES += X11FontScaler.c \ X11TextRenderer.c \ fontpath.c \ lcdglyph.c else LIBFONTMANAGER_EXCLUDE_FILES += fontpath.c \ lcdglyph.c endif The X11*.c files are in the unix directory and the fontpath/lcdglyph files are in the windows directory, so they need to explicit excludes. However, the X11 files stills needs to be excluded on macosx. On the other hand, these two are all that exists in $(JDK_TOPDIR)/src/java.desktop/$(OPENJDK_TARGET_OS_API_DIR)/native/libfontmanager, so we can just exclude that entire directory, or only add it unless we're building for macosx. libkcms: EXCLUDE_FILES := $(BUILD_LIBKCMS_EXCLUDE_FILES), is not needed, since BUILD_LIBKCMS_EXCLUDE_FILES is no longer defined. libjawt: INCLUDE_FILES := $(LIBJAWT_INCLUDE_FILES), and INCLUDE_FILES := $(JAWT_FILES), are not needed, since LIBJAWT_INCLUDE_FILES and JAWT_FILES are not defined. libawt_lwawt: INCLUDE_FILES := $(LIBAWT_LWAWT_FILES), is not needed, since LIBAWT_LWAWT_FILES is not defined. libt2k: According to EXCLUDE_FILES := t2k/orion.c, the file orion.c is never used, so it should be removed instead. Also, the entire library definition should move to closed code. liblcms: The include file picking of LCMS.c would not be needed if the upstream lcms source code were moved into a separate subdirectory of liblcms. libjavajpeg: If the upstream IJG jpeg library were moved into a subdirectory, the explicit includes of imageIOJPEG.c and jpegdecoder.c would not be needed. libmlib_image / libmlib_image_v: The files used for libmlib_image and libmlib_image_v are somewhat chaotic. Four directories are used, in different ways: 1) SHARE-LIB: java.desktop/share/native/libmlib_image 2) UNIX-LIB: java.desktop/unix/native/libmlib_image 3) SHARE-COMMON: java.desktop/share/native/common/sun/awt/medialib 4) UNIX-COMMON: java.desktop/unix/native/common/sun/awt/medialib They are used to build three different libraries: A) libmlib_image. Built on all platforms. Includes SHARE-LIB and SHARE-COMMON B) libmlib_image_v. Built on solaris-sparc only. Includes a subset (using excludes) of SHARE-LIB, UNIX-LIB, SHARE-COMMON and UNIX-COMMON. C) libawt. Uses SHARE-COMMON and UNIX-COMMON, but only when building for solaris-sparc. Maybe this complexity is not needed, but even if it is, there are a number of things that could help improve the situation. * The UNIX-LIB and UNIX-COMMON paths are only used on solaris and should move from "unix" to "solaris". * The SHARE-COMMON and UNIX-COMMON paths should move away from the common/sun/awt directories, since they need to be explicitely excluded by all other users of the common/sun/awt directories, e.g. to common/mlib_image. This would reduce the number of excludes elsewhere significantly. * The code in UNIX-LIB is used to build libmlib_image_v, not libmlib_image, and should thus be renamed so. * The code in SHARE-LIB that is actually shared by libmlib_image and libmlib_image_v should preferably be moved to java.desktop/share/native/common. I don't have any good suggestions though on how this should be handled so as not to collide with the different shared subset found in SHARE-COMMON. These files include basically the mlib_Image*.c files. It also includes the file mlib_c_ImageThresh1_U8.c, while excluding all other mlib_c_*.c. This almost looks like a mistake. * A number of files are hardcoded to always be excluded. These should be removed instead. These files are: In java.desktop/share/native/libmlib_image: mlib_c_ImageBlendTable.c In java.desktop/unix/native/libmlib_image: mlib_v_ImageChannelExtract.c \ mlib_v_ImageChannelExtract_f.c \ mlib_v_ImageChannelInsert_34.c \ mlib_v_ImageChannelInsert.c \ mlib_v_ImageConvIndex3_8_16nw.c \ mlib_v_ImageCon
Re: RFR [9] Modular Source Code
On 8/15/14, 3:13 AM, Erik Joelsson wrote: * GensrcCLDR.gmk is not ideal. It generates source for multiple modules, and the output is separated afterwards. Fixing this will probably means some modification to the java helper tools. While I think it would be good to split this in principle, I'm not sure it's worth pursuing. The english locale is in the base module and the others are in a different module. I would leave this as it is for now. I agree with Erik, as I will be working on this later. So let's leave it as is. Naoto
Re: RFR [9] Modular Source Code
Yes, we (the LangTools team) are on our own for this file. Jan Lahoda did some work to update the build.xml file for us. At some point we may want to do more surgery on this file, but that is a discussion for the LangTools team to have. -- Jon On 08/15/2014 06:17 AM, Erik Joelsson wrote: On 2014-08-15 14:54, Maurizio Cimadamore wrote: I'm looking at the langtools-related changes in build.xml; I what is the degree of support available in the build.xml ant file for the Jigsaw world? It seems to me that not all the target would function and it also seems that some of the properties previously encoded in a side property file (make/build.properties) have now been inlined in the build.xml file itself, which seems problematic maintenance-wise. Am I missing something? As far as I know, build.xml has so far been supported by the langtools team themselves and all changes to the file in this patch have been made by them. Jon may have something to say? /Erik Maurizio On 12/08/14 15:10, Chris Hegarty wrote: This is a review request for the Initial changes for JEP 201: Modular Source Code [1]. There are a number of individuals responsible for these changes. Some, possibly not all, are explicitly listed in the To section of this mail, and they will help address any comments arising from this review request. For the purposes of review, the actual source file moves have been omitted from the webrev below, with the exception of any source file that has a change to it’s actual content. The new location of the source files can be determined from JEP 201 [1] and JEP 200 "The Modular JDK" [2], or by browsing the staging forest [3]. Webrevs: http://cr.openjdk.java.net/~chegar/8054834/00/ Due to the significant impact of these changes, a JDK 9 promotion has been tentatively reserved for their integration. All comments are welcome, although given the nature of the changes then we might have to create separate issues in JIRA to address some of them later in jdk9/dev.. -Chris. [1] https://bugs.openjdk.java.net/browse/JDK-8051619 [2] https://bugs.openjdk.java.net/browse/JDK-8051618 [3] http://hg.openjdk.java.net/jigsaw/stage
Re: RFR [9] Modular Source Code
On 8/12/2014 7:10 AM, Chris Hegarty wrote: Webrevs: http://cr.openjdk.java.net/~chegar/8054834/00/ I reviewed the hotspot change. Looks good. One minor point: I think line 1230 can be removed when rt.jar is present. Mandy
Re: RFR [9] Modular Source Code
Hello Omair, No, as I tried to explain in my initial mail on this thread, since the modules are all compiled in parallel, and not sequentially like the current build does with the repositories, it doesn't make much sense timing each module. The numbers would be meaningless. /Erik On 2014-08-15 17:55, Omair Majid wrote: Hi, * Alan Bateman [2014-08-13 08:36]: Just to add to Chris and Erik's mails, I would encourage everyone that pushes to jdk9/dev or the other jdk9 project integration forests to clone and build jigsaw/stage and get familiar with the proposed layout, new build targets, and the very different output emitted during the build. The changes are arguably as significant as the transition in JDK 8 from the "old build" to the "new build" so the more people taking the forest for a test drive the better. If you maintain your our own own IDE project then you'll likely have to adjust file paths so any issues encountered would be useful to hear about too. Just one RFE for now: The new build would provide a detailed breakdown of the build time of each repo. With the 'new new build', all I see is an overall time: - Build times --- Start 2014-08-15 10:59:27 End 2014-08-15 11:17:53 00:18:26 TOTAL - Will a detailed breakdown of the build times for each module make a return? Thanks, Omair
Re: RFR [9] Modular Source Code
Hi, * Alan Bateman [2014-08-13 08:36]: > Just to add to Chris and Erik's mails, I would encourage everyone that > pushes to jdk9/dev or the other jdk9 project integration forests to clone > and build jigsaw/stage and get familiar with the proposed layout, new build > targets, and the very different output emitted during the build. The changes > are arguably as significant as the transition in JDK 8 from the "old build" > to the "new build" so the more people taking the forest for a test drive the > better. If you maintain your our own own IDE project then you'll likely have > to adjust file paths so any issues encountered would be useful to hear about > too. Just one RFE for now: The new build would provide a detailed breakdown of the build time of each repo. With the 'new new build', all I see is an overall time: > - Build times --- > Start 2014-08-15 10:59:27 > End 2014-08-15 11:17:53 > > 00:18:26 TOTAL > - Will a detailed breakdown of the build times for each module make a return? Thanks, Omair -- PGP Key: 66484681 (http://pgp.mit.edu/) Fingerprint = F072 555B 0A17 3957 4E95 0056 F286 F14F 6648 4681
Re: RFR [9] Modular Source Code
On 8/15/2014 1:49 AM, Magnus Ihse Bursie wrote: *** Issues regarding modules.xml *** The new modules.xml and associated Java tools and make files seems rather confusing to me. I understand some of the mysteries here are due the the fact that the file has moved around during development. Nevertheless, such historical relics should be removed when the code is ready to be pushed to mainline. More concretely: One thing to say about the modules.xml file is that it's not just for jdeps to use (verifying the module boundaries). It is an essential documentation to describe the module definition for the JDK [1] and will go away once the module system is in place and not all the modules are in the jdk repo and hence it's placed in the top level repo. * ModulesXml.gmk referes to make/data/checkdeps/modules.xml. This file does not exist. I believe this should be the $(TOPDIR)/modules.xml. That reference in the comment that we missed to update when moving the file. Fixed. Mandy [1] https://bugs.openjdk.java.net/browse/JDK-8051618
Re: RFR [9] Modular Source Code
Quick update: I downloaded the jigsaw stage repo and tried the build. The biggest issue is that it requires Ant 1.9.4 (because of the MuliRootFileSet) to work, but after that is set up, everything works like before. My IntelliJ setup needed few cosmetic changes, to update the source roots, but nothing out of the ordinary. Overall, I'm pretty satisfied with it. Maurizio On 15/08/14 14:17, Erik Joelsson wrote: On 2014-08-15 14:54, Maurizio Cimadamore wrote: I'm looking at the langtools-related changes in build.xml; I what is the degree of support available in the build.xml ant file for the Jigsaw world? It seems to me that not all the target would function and it also seems that some of the properties previously encoded in a side property file (make/build.properties) have now been inlined in the build.xml file itself, which seems problematic maintenance-wise. Am I missing something? As far as I know, build.xml has so far been supported by the langtools team themselves and all changes to the file in this patch have been made by them. Jon may have something to say? /Erik Maurizio On 12/08/14 15:10, Chris Hegarty wrote: This is a review request for the Initial changes for JEP 201: Modular Source Code [1]. There are a number of individuals responsible for these changes. Some, possibly not all, are explicitly listed in the To section of this mail, and they will help address any comments arising from this review request. For the purposes of review, the actual source file moves have been omitted from the webrev below, with the exception of any source file that has a change to it’s actual content. The new location of the source files can be determined from JEP 201 [1] and JEP 200 "The Modular JDK" [2], or by browsing the staging forest [3]. Webrevs: http://cr.openjdk.java.net/~chegar/8054834/00/ Due to the significant impact of these changes, a JDK 9 promotion has been tentatively reserved for their integration. All comments are welcome, although given the nature of the changes then we might have to create separate issues in JIRA to address some of them later in jdk9/dev.. -Chris. [1] https://bugs.openjdk.java.net/browse/JDK-8051619 [2] https://bugs.openjdk.java.net/browse/JDK-8051618 [3] http://hg.openjdk.java.net/jigsaw/stage
Re: RFR [9] Modular Source Code
On 15/08/2014 09:49, Magnus Ihse Bursie wrote: These are indeed the single most significant changes to the build system since the "new" build system was introduced. Indeed, some of us have been referring to this as the "new new build". As I recall there was a tail of issues and clean-ups for the JDK 8 "new build" and it will be the case here too. So if there aren't any blocking issues then I think the best thing is to work on those in jdk9/dev once the changes get there, otherwise I suspect we will iterate on this for a long time (and as I'm sure you can appreciate, it is painful to have to keep thousands of lines of make file changes in a side forest while the main line churns). : * When the source code has moved, especially the native libraries, most of the specific INCLUDE and EXCLUDE statements are, or should be, unnecessary. Nevertheless, there are still several occasions of such statements. In some cases, it seems like dead code that can (and should) be removed. But in some cases, I believe it is an indication that the source code has still not yet been moved to a suitable location. I believe the end goal with this shuffling regarding native library source code is that there should be a one-to-one correspondance between compiled native library and source code directory. This is now indeed the case for 99% of the libraries, but there are still some exceptions. I assume you are referring to the rename of src/solaris to src/unix, something that was long overdue in the jdk repository. The intention is that eventually each area will go through their source files in src/$MODULE/unix and move any source files that are Linux, Solaris or OS X specific to the appropriate src/$MODULE/$OS tree. We've done it for a few libraries (such as libnio) so several EXCLUDE_FILES are already removed, the remainder of the exceptions will happen in time. : * Finally, I'm also not entirely happy with the placement of modules.xml in the root directory. Erik has told me that the intention is that this file will ultimately be created dynamically at build time, and when that happens, it will just be a build by-product which can be placed elsewhere. If this is indeed the case, I can live with some temporary extra cluttering of the top-level directory The top-level directory is intentional. It's not worth getting hung up on it because it is temporary (as noted in the JEP) and so will go away once we are further down the road on having a module system in JDK 9. -Alan.
Re: RFR [9] Modular Source Code
I'm looking at the langtools-related changes in build.xml; I what is the degree of support available in the build.xml ant file for the Jigsaw world? It seems to me that not all the target would function and it also seems that some of the properties previously encoded in a side property file (make/build.properties) have now been inlined in the build.xml file itself, which seems problematic maintenance-wise. Am I missing something? Maurizio On 12/08/14 15:10, Chris Hegarty wrote: This is a review request for the Initial changes for JEP 201: Modular Source Code [1]. There are a number of individuals responsible for these changes. Some, possibly not all, are explicitly listed in the To section of this mail, and they will help address any comments arising from this review request. For the purposes of review, the actual source file moves have been omitted from the webrev below, with the exception of any source file that has a change to it’s actual content. The new location of the source files can be determined from JEP 201 [1] and JEP 200 "The Modular JDK" [2], or by browsing the staging forest [3]. Webrevs: http://cr.openjdk.java.net/~chegar/8054834/00/ Due to the significant impact of these changes, a JDK 9 promotion has been tentatively reserved for their integration. All comments are welcome, although given the nature of the changes then we might have to create separate issues in JIRA to address some of them later in jdk9/dev.. -Chris. [1] https://bugs.openjdk.java.net/browse/JDK-8051619 [2] https://bugs.openjdk.java.net/browse/JDK-8051618 [3] http://hg.openjdk.java.net/jigsaw/stage
Re: RFR [9] Modular Source Code
On 2014-08-15 14:54, Maurizio Cimadamore wrote: I'm looking at the langtools-related changes in build.xml; I what is the degree of support available in the build.xml ant file for the Jigsaw world? It seems to me that not all the target would function and it also seems that some of the properties previously encoded in a side property file (make/build.properties) have now been inlined in the build.xml file itself, which seems problematic maintenance-wise. Am I missing something? As far as I know, build.xml has so far been supported by the langtools team themselves and all changes to the file in this patch have been made by them. Jon may have something to say? /Erik Maurizio On 12/08/14 15:10, Chris Hegarty wrote: This is a review request for the Initial changes for JEP 201: Modular Source Code [1]. There are a number of individuals responsible for these changes. Some, possibly not all, are explicitly listed in the To section of this mail, and they will help address any comments arising from this review request. For the purposes of review, the actual source file moves have been omitted from the webrev below, with the exception of any source file that has a change to it’s actual content. The new location of the source files can be determined from JEP 201 [1] and JEP 200 "The Modular JDK" [2], or by browsing the staging forest [3]. Webrevs: http://cr.openjdk.java.net/~chegar/8054834/00/ Due to the significant impact of these changes, a JDK 9 promotion has been tentatively reserved for their integration. All comments are welcome, although given the nature of the changes then we might have to create separate issues in JIRA to address some of them later in jdk9/dev.. -Chris. [1] https://bugs.openjdk.java.net/browse/JDK-8051619 [2] https://bugs.openjdk.java.net/browse/JDK-8051618 [3] http://hg.openjdk.java.net/jigsaw/stage
Re: RFR [9] Modular Source Code
On 15/08/2014 11:13, Magnus Ihse Bursie wrote: : In NetworkingLibraries.gmk: * There are multiple instances of this pattern: ifneq ($(OPENJDK_TARGET_OS), solaris) LIBNET_EXCLUDE_FILES += solaris_close.c endif The correct solution is to move the corresponding files away from the "unix" directory and into more specific libraries (linux, solaris and macosx) and include these directories automatically depending on platform. This will allow us to remove the exclude expression. It's good that you giving these changes a thorough review. I agree that this and all the other $OS specific source that you have listed move and the EXCLUDES dropped. As I said in the previous reply then this is not necessary for this initial push. Instead the intention was to establish the new layout and for each area to gradually move any remaining Solaris, Linux and OS X sources from src/$MODULE/unix to src/$MODULE/$OS. It does mean that about 1% of the source files will need to move again once the changes are in jdk9/dev but I don't expect this to be disruptive. -Alan.
Re: RFR [9] Modular Source Code
Hello, Magnus Thanks for the thorough review. I tend to agree with Alan, that we should rather push this in unless we find critical issues but make sure to continue cleaning this up in jdk9/dev. I have created bugs for (almost) everything you have listed below. See inline for links and comments. On 2014-08-15 10:52, Magnus Ihse Bursie wrote: Here follows a partial review of the changes in the jdk repo. Even though it's long, I'm not done. ;-) *** General issues *** * The new directory jdk/make/bundle should instead reside in jdk/make/data/bundles. * In GensrcSwing.gmk is a "$(if $(SHUFFLED)..." part that seems to be remnants of older, temporary work. Created " General cleanup of minor issues from source restructure" https://bugs.openjdk.java.net/browse/JDK-8055188 * The new file GensrcProviders.gmk are included by the Gensrc files for jdk.attach and jdk.jdi, which only uses half of it each. Each half is just a few lines long. I believe a better solution is to remove the GensrcProviders.gmk file, move the process-provider macro to GensrcCommon.gmk and move the two actual rules to the respective module gensrc file. * In the gensrc directory, there are now almost twice as many files. For many of them, the following pattern holds: GensrcOldStyle.gmk -- defines the actual logic for some gensrc target Gensrc-.gmk -- does nothing but includes GensrcOldStyle.gmk In these cases, I think the contents of GensrcOldStyle should be inlined directly in the Gensrc-.gmk instead. This holds for all modules except the two mammuth modules java.base and java.desktop. (Depending on the treatment of GensrcProviders as described above.) Created "Cleanup gensrc after source code restructure" https://bugs.openjdk.java.net/browse/JDK-8055189 * In CreateJars.gmk, the following new comment is found embedded: # This currently won't work with modular build layout, but there currently are no # types needing to be re added. In my opinion, leaving code which looks like it's working but with a note saying "out of order", is bad practice. I'd recommend that the code is removed or commented out, if it is not needed. While I agree on this in principle, I would rather not spend time on the profiles generation code since it's planned to go away soon anyway. * In CreateJars.gmk, in BUILD_TOOLS_JAR: The following looks like a duplication; I believe the "classes" version should be removed. $(JDK_OUTPUTDIR)/modules/jdk.jdi/META-INF/services/com.sun.jdi.connect.Connector \ $(JDK_OUTPUTDIR)/classes/META-INF/services/com.sun.jdi.connect.Connector \ * In CreateSecurityJars.gmk, there is a variable SECURITY_CLASSES_SUBDIR that is always set to 'modules'. I believe this is remains of an older temporary design, and that "modules" should be hard-coded into the paths. * The old Setup.gmk had a very generic-sounding name, even though it only did setup java compilers. So, the rename to SetupJava.gmk was good; however, I'd suggest we follow it through all the way and rename it further to SetupJavaCompilers.gmk, since that is an even more accurate description of it's job. * The file CopyIntoClasses.gmk is not used anymore and should be removed. * In CoreLibraries.gmk, there used to be a line "BUILD_LIBRARIES += $(BUILD_LIBFDLIBM)" which is now removed. After discussion with Erik, I learned that this was since the libfdlibm was not delivered in itself, but was used solely as a helper lib for libjava, which has BUILD_LIBFDLIBM as a requirement. While this change is thus correct, I believe a comment describing this fact would be in place on libfdlibm, since it makes it behave differently than all other libraries. Added to general cleanup *** Issues with source files moving and its repercussions *** * When the source code has moved, especially the native libraries, most of the specific INCLUDE and EXCLUDE statements are, or should be, unnecessary. Nevertheless, there are still several occasions of such statements. In some cases, it seems like dead code that can (and should) be removed. But in some cases, I believe it is an indication that the source code has still not yet been moved to a suitable location. I believe the end goal with this shuffling regarding native library source code is that there should be a one-to-one correspondance between compiled native library and source code directory. This is now indeed the case for 99% of the libraries, but there are still some exceptions. This is a slightly vague point at the moment. I indent to check the INCLUDE and EXCLUDE statements more fully and will post a second review with results of what I find. Nevertheless, I think it is important to make sure we do get things correct this time. Created "Cleanup include and exclude of native libraries after source code restructure" https://bugs.openjdk.java.net/browse/JDK-8055190 *** Issues regarding modules.xml *** The new modules.xml and associated Java tools and
Re: RFR [9] Modular Source Code
On 2014-08-15 10:52, Magnus Ihse Bursie wrote: *** Issues with source files moving and its repercussions *** * When the source code has moved, especially the native libraries, most of the specific INCLUDE and EXCLUDE statements are, or should be, unnecessary. Nevertheless, there are still several occasions of such statements. In some cases, it seems like dead code that can (and should) be removed. But in some cases, I believe it is an indication that the source code has still not yet been moved to a suitable location. I believe the end goal with this shuffling regarding native library source code is that there should be a one-to-one correspondance between compiled native library and source code directory. This is now indeed the case for 99% of the libraries, but there are still some exceptions. This is a slightly vague point at the moment. I indent to check the INCLUDE and EXCLUDE statements more fully and will post a second review with results of what I find. Nevertheless, I think it is important to make sure we do get things correct this time. Here are the more concrete specification of this, for all files except Awt2dLibraries.gmk, which I'll return to. In NioLibraries: * The line "EXCLUDES := sctp" is unnecessary. In NetworkingLibraries.gmk: * There are multiple instances of this pattern: ifneq ($(OPENJDK_TARGET_OS), solaris) LIBNET_EXCLUDE_FILES += solaris_close.c endif The correct solution is to move the corresponding files away from the "unix" directory and into more specific libraries (linux, solaris and macosx) and include these directories automatically depending on platform. This will allow us to remove the exclude expression. * For AIX, this is already done (woho!); however, unfortunately, the file aix_close.c ended up not in java.base/aix/native/libnet/ but in java.base/aix/native/libnet/java/net, with remnants of the old directory structure still intact. * Also, the corresponding source line in NetworkingLibraries.gmk for AIX is incorrect, and refers to the old structure: LIBNET_SRC_DIRS += $(JDK_TOPDIR)/src/aix/native/java/net/ is incorrect. But if solving the two problems above, this will be corrected all by itself, rendering this statement unnecessary. In Lib-jdk.security.auth.gmk: * The file src/jdk.security.auth/unix/native/libjaas/Solaris.c should move to a solaris directory instead, rendering the EXCLUDE for libjaas unnecessary. In Lib-jdk.attach.gmk: The files * src/jdk.attach/unix/native/libattach/LinuxVirtualMachine.c * src/jdk.attach/unix/native/libattach/SolarisVirtualMachine.c * src/jdk.attach/unix/native/libattach/BsdVirtualMachine.c should move from unix to linux, solaris and macosx respectively, rendering the EXCLUDES unnecessary. The statement LIBATTACH_EXCLUDE_FILES += AixVirtualMachine.c is already unnecessary, since that files virtuously is already placed in an aix directory! :-) In Lib-java.management.gmk: The files * src/java.management/unix/native/libmanagement/LinuxOperatingSystem.c * src/java.management/unix/native/libmanagement/SolarisOperatingSystem.c * src/java.management/unix/native/libmanagement/MacosxOperatingSystem.c should move from unix to linux, solaris and macosx respectively, rendering the EXCLUDES unnecessary. In CoreLibraries.gmk, for libjava: * The file src/java.base/unix/native/libjava/java_props_macosx.c should move to a macosx directory. * The line "EXCLUDES := fdlibm/src zip prefs" is not needed anymore. * The stanza: ifeq ($(OPENJDK_TARGET_OS), macosx) LIBJAVA_EXCLUDE_FILES += $(JDK_TOPDIR)/src/java.base/unix/native/libjava/HostLocaleProviderAdapter_md.c endif is unnecessary since no such file exists. In CoreLibraries.gmk, for libjli: Here are include statements galore! :) After parsing what we're supposed to do and checking with how the source code now actually looks, this can boil down to: * As normal, set the source dirs to share and PLATFORM_OS and OS_API. * On macosx, exclude java_md_solinux.c, ergo.c and ergo_i586.c. * On unixes that are not macosx: If OPENJDK_TARGET_CPU_ARCH != x86 then also exclude ergo_i586.c and set LIBJLI_CFLAGS += -DUSE_GENERIC_ERGO But to make things worse, we also use a selected subset of the source from zlib :-(, viz.: inflate.c inftrees.c inffast.c zadler32.c zcrc32.c zutil.c This should be turned into a exclude-based statement instead (unless USE_EXTERNAL_LIBZ is true, of course), like this: BUILD_LIBJLI_SRC_DIRS += $(JDK_TOPDIR)/src/java.base/share/native/libzip/zlib-1.2.8 (as before) and exclude: compress.c deflate.c gzclose.c gzlib.c gzread.c gzwrite.c infback.c trees.c uncompr.c /Magnus
Re: RFR [9] Modular Source Code
On 2014-08-12 16:10, Chris Hegarty wrote: This is a review request for the Initial changes for JEP 201: Modular Source Code [1]. There are a number of individuals responsible for these changes. Some, possibly not all, are explicitly listed in the To section of this mail, and they will help address any comments arising from this review request. For the purposes of review, the actual source file moves have been omitted from the webrev below, with the exception of any source file that has a change to it’s actual content. The new location of the source files can be determined from JEP 201 [1] and JEP 200 "The Modular JDK" [2], or by browsing the staging forest [3]. Webrevs: http://cr.openjdk.java.net/~chegar/8054834/00/ These are indeed the single most significant changes to the build system since the "new" build system was introduced. Here follows a partial review of the changes in the jdk repo. Even though it's long, I'm not done. ;-) *** General issues *** * The new directory jdk/make/bundle should instead reside in jdk/make/data/bundles. * In GensrcSwing.gmk is a "$(if $(SHUFFLED)..." part that seems to be remnants of older, temporary work. * The new file GensrcProviders.gmk are included by the Gensrc files for jdk.attach and jdk.jdi, which only uses half of it each. Each half is just a few lines long. I believe a better solution is to remove the GensrcProviders.gmk file, move the process-provider macro to GensrcCommon.gmk and move the two actual rules to the respective module gensrc file. * In the gensrc directory, there are now almost twice as many files. For many of them, the following pattern holds: GensrcOldStyle.gmk -- defines the actual logic for some gensrc target Gensrc-.gmk -- does nothing but includes GensrcOldStyle.gmk In these cases, I think the contents of GensrcOldStyle should be inlined directly in the Gensrc-.gmk instead. This holds for all modules except the two mammuth modules java.base and java.desktop. (Depending on the treatment of GensrcProviders as described above.) * In CreateJars.gmk, the following new comment is found embedded: # This currently won't work with modular build layout, but there currently are no # types needing to be re added. In my opinion, leaving code which looks like it's working but with a note saying "out of order", is bad practice. I'd recommend that the code is removed or commented out, if it is not needed. * In CreateJars.gmk, in BUILD_TOOLS_JAR: The following looks like a duplication; I believe the "classes" version should be removed. $(JDK_OUTPUTDIR)/modules/jdk.jdi/META-INF/services/com.sun.jdi.connect.Connector \ $(JDK_OUTPUTDIR)/classes/META-INF/services/com.sun.jdi.connect.Connector \ * In CreateSecurityJars.gmk, there is a variable SECURITY_CLASSES_SUBDIR that is always set to 'modules'. I believe this is remains of an older temporary design, and that "modules" should be hard-coded into the paths. * The old Setup.gmk had a very generic-sounding name, even though it only did setup java compilers. So, the rename to SetupJava.gmk was good; however, I'd suggest we follow it through all the way and rename it further to SetupJavaCompilers.gmk, since that is an even more accurate description of it's job. * The file CopyIntoClasses.gmk is not used anymore and should be removed. * In CoreLibraries.gmk, there used to be a line "BUILD_LIBRARIES += $(BUILD_LIBFDLIBM)" which is now removed. After discussion with Erik, I learned that this was since the libfdlibm was not delivered in itself, but was used solely as a helper lib for libjava, which has BUILD_LIBFDLIBM as a requirement. While this change is thus correct, I believe a comment describing this fact would be in place on libfdlibm, since it makes it behave differently than all other libraries. *** Issues with source files moving and its repercussions *** * When the source code has moved, especially the native libraries, most of the specific INCLUDE and EXCLUDE statements are, or should be, unnecessary. Nevertheless, there are still several occasions of such statements. In some cases, it seems like dead code that can (and should) be removed. But in some cases, I believe it is an indication that the source code has still not yet been moved to a suitable location. I believe the end goal with this shuffling regarding native library source code is that there should be a one-to-one correspondance between compiled native library and source code directory. This is now indeed the case for 99% of the libraries, but there are still some exceptions. This is a slightly vague point at the moment. I indent to check the INCLUDE and EXCLUDE statements more fully and will post a second review with results of what I find. Nevertheless, I think it is important to make sure we do get things correct this time. *** Issues regarding modules.xml *** The new modules.xml and associated Java tools and make files seems rather co
Re: RFR [9] Modular Source Code
On 2014-08-14 09:07, Erik Joelsson wrote: - javadoc.gmk: JAVADOC_CMD should perhaps use (currently non-existant) JAVA_TOOL_FLAGS_BIG or at least JAVA_FLAGS_BIG? I agree, but that should be a separate change. Actually, the variable JAVA already contains the big java flags. The correct fix here is to remove the redundant explicit mx flag from Javadoc.gmk and just trust that JAVA is correctly sized. /Erik
Re: RFR [9] Modular Source Code
Hello Mike, Thanks for the comments. See inline. On 2014-08-13 23:29, Mike Duigou wrote: There's a lot to review here. This is not a complete review but hopefully contributes to our review "coverage". I am focusing on the top project in this set of comments. - --with-output-sync seems like it should be on by default if available. Downside? This could also be split out from the jigsaw changes if there is any interest in reducing the patch size. There are downsides yes. The way this works is that make will buffer output from all commands and print them when done. Depending on level that's when the command line is done, the recipe is done, or the complete submake. I played around with having it default for a while. For a normal build this isn't too confusing once you get used to it, but it really made running tests annoying as the output from jtreg would all be buffered until it was done. Also, a hotspot developer that was to build hotspot from the root repo would see nothing until the whole build was complete. I simply believe we need some more experience with this before we can decide on a better default behavior. Possibly we need to do the hotspot makefile rewrite first so that the hotspot build can be split into smaller logical chunks from the top level. I wanted the output-sync feature to be in this from the start to make JPRT logs easier to look at, as the feature will be default turned on there. Especially debug logs become very hard to read without it. - what is TESTMAKE_OUTPUTDIR for? (ugh, more outputdir dirs...) While doing this work, I had the need for adding features to SetupArchive (mainly to support multiple source dirs for jars). It quickly turned quite nasty and to make development easier, I added specific test cases for this macro in a separate makefile structure. This is the output directory for those tests. - spec.gmk.in: Can we have a separate assignment for JAVA_TOOL_FLAGS_SMALL? It is nice to be able to see every AC_SUBST somewhere solo. Certainly, that makes sense. Must have just missed it. - jdk-options.m4: should with_cacerts_file being empty not merit an error? what does the empty default do? The default is to use the bundled one. I didn't like having that default being defined in configure. I think it better belongs in the makefile handling the file. - javadoc.gmk: retire JDK_IMPSRC, JDK_GENSRC, JDK_SHARE_CLASSES and JDK_SHARE_SRC We should certainly clean up javadoc.gmk properly, but I thought that was out of scope for this patch. You do have a point in that those variables are now unused so no longer need to be declared. - javadoc.gmk: JAVADOC_CMD should perhaps use (currently non-existant) JAVA_TOOL_FLAGS_BIG or at least JAVA_FLAGS_BIG? I agree, but that should be a separate change. - MakeHelpers: CleanComponent should call strip on the $1 argument to $(RM) so that it is deleting what it promises to be deleting. Or it could check to make sure $(words $1) is 1 Strip clears leading and trailing whitespace. The reason I added a strip to the echo is that in some cases the macro was called with a space after comma and in some not. The output looked weird when the echo line sometimes had 2 spaces before the component name instead of one. It should not matter to the rm command line however. - modules.xml "Changes to this file will require review by Committers to Project Jigsaw." Will this be true after integration into jdk9/dev repo? - modules.list seems to be redundant with modules.xml but there don't seem to be any measures to ensure that they remain in sync. Even a comment in modules.xml would help. This kind of problem has been a source of errors in the past. They are redundant yes. modules.list is used by make to extract dependency information and modules.xml is used by the verification tool. In jigsaw development both files were dynamically created during the build process and here we simply committed static versions of them. Ideally we should only need one, but it's a temporary solution anyway. We would need to create a tool to extract dependency information from the xml to make. - What is TestMake.gmk and associated for? These are my new tests for the common makefile logic. It's far from a complete coverage, but it has already helped me greatly. jdk project: - I am slightly unsettled by the number of makefiles and putting them all in to the same directory. Will they eventually be moved into their modules? This is something I'm thinking about and would like to discuss. I think that ideally the makefiles for specific modules should be moved to module specific directories. For now I kept the existing task based directory structure. More to come but first I want to build it! Go for it! /Erik On Aug 12 2014, at 07:10 , Chris Hegarty wrote: This is a review request for the Initial changes for JEP 201: Modular Source Code [1]. There are a number of individuals responsible for these changes. Some, possibly
Re: RFR [9] Modular Source Code
On 13/08/2014 22:29, Mike Duigou wrote: : - modules.xml "Changes to this file will require review by Committers to Project Jigsaw." Will this be true after integration into jdk9/dev repo? There's a section in JEP 200 about modules.xml, it is temporary until there is a module system in place. The jdeps tool is updated with a new option to read it and check for changes (the dependency check is more detailed that might be initially obvious). For now, committers to Project Jigsaw are the custodians of the module graph and I think the requirement for a Reviewer will last at least as long as this side file needs to exist. In time I assume that processes for dealing with changes to the module graph will be on par with other API changes. -Alan
Re: RFR [9] Modular Source Code
There's a lot to review here. This is not a complete review but hopefully contributes to our review "coverage". I am focusing on the top project in this set of comments. - --with-output-sync seems like it should be on by default if available. Downside? This could also be split out from the jigsaw changes if there is any interest in reducing the patch size. - what is TESTMAKE_OUTPUTDIR for? (ugh, more outputdir dirs...) - spec.gmk.in: Can we have a separate assignment for JAVA_TOOL_FLAGS_SMALL? It is nice to be able to see every AC_SUBST somewhere solo. - jdk-options.m4: should with_cacerts_file being empty not merit an error? what does the empty default do? - javadoc.gmk: retire JDK_IMPSRC, JDK_GENSRC, JDK_SHARE_CLASSES and JDK_SHARE_SRC - javadoc.gmk: JAVADOC_CMD should perhaps use (currently non-existant) JAVA_TOOL_FLAGS_BIG or at least JAVA_FLAGS_BIG? - MakeHelpers: CleanComponent should call strip on the $1 argument to $(RM) so that it is deleting what it promises to be deleting. Or it could check to make sure $(words $1) is 1 - modules.xml "Changes to this file will require review by Committers to Project Jigsaw." Will this be true after integration into jdk9/dev repo? - modules.list seems to be redundant with modules.xml but there don't seem to be any measures to ensure that they remain in sync. Even a comment in modules.xml would help. This kind of problem has been a source of errors in the past. - What is TestMake.gmk and associated for? jdk project: - I am slightly unsettled by the number of makefiles and putting them all in to the same directory. Will they eventually be moved into their modules? More to come but first I want to build it! On Aug 12 2014, at 07:10 , Chris Hegarty wrote: > This is a review request for the Initial changes for JEP 201: Modular Source > Code [1]. > > There are a number of individuals responsible for these changes. Some, > possibly not all, are explicitly listed in the To section of this mail, and > they will help address any comments arising from this review request. > > For the purposes of review, the actual source file moves have been omitted > from the webrev below, with the exception of any source file that has a > change to it’s actual content. The new location of the source files can be > determined from JEP 201 [1] and JEP 200 "The Modular JDK" [2], or by browsing > the staging forest [3]. > > Webrevs: > http://cr.openjdk.java.net/~chegar/8054834/00/ > > Due to the significant impact of these changes, a JDK 9 promotion has been > tentatively reserved for their integration. All comments are welcome, > although given the nature of the changes then we might have to create > separate issues in JIRA to address some of them later in jdk9/dev.. > > -Chris. > > [1] https://bugs.openjdk.java.net/browse/JDK-8051619 > [2] https://bugs.openjdk.java.net/browse/JDK-8051618 > [3] http://hg.openjdk.java.net/jigsaw/stage
Re: RFR [9] Modular Source Code
Thanks for the explanation Erik. I have taken a pass over the changes, and they look ok to me ( I am happy to be listed as a reviewer ). I also did several build and test runs on Solaris, Linux, Max OSX, and Windows. All look good. I am seeing, in some cases, about a 20% reduction in image build times on an 8 core i7, running Linux x86. One question; Are there any new requirements on build systems as a result of these changes? -Chris. On 13/08/14 13:03, Erik Joelsson wrote: I should probably write something about the rather extensive changes to the build logic in this patch. As the source gets organized around modules, it made sense to also organize the build more around modules. In this patch, the makefiles have in large parts been split into module specific files and the top level targets are oriented around modules. This means the top level targets are much more fine granular than currently in JDK 9. Another difference is that the top level targets are now able to run concurrently, to give more opportunity for utilizing multiple cpus. In JDK 9, the build moves sequentially through each repository, in a given order, now make is free to run more things in parallel. This makes the build faster, at least on beefy hardware. The drawback is that the build log gets a bit more confusing. When something fails, it won't interrupt other building threads at once, so the actual failure may be further up the log (even further than before). I have found that searching for "Error 2" is a good way to find the real failure. Another consequence is that the build time summary at the end only displays total time as there is no good definition for how much time was spent in each repository anymore. A summary on the new targets: make [default] Does pretty much the same as before. It compiles everything but does not build all jars or create images. make all Builds everything, including jars, images and docs. Also runs a verification tool on the java classes which will point out any broken module boundaries. make images Same as before make hotspot Builds the hotspot repository, like before. make docs Builds all the documentation, including javadoc. make docs-javadoc Builds just the javadoc. This target has very few prerequisites so provides a fast way to just build javadoc. make gensrc Runs all source code generation steps. make java Compiles all java classes. Other similar targets are libs, launchers, gendata and copy make java.desktop Compiles everything in the java.desktop module (and its dependencies), both java and native code. Works for any module name. make java.desktop-java Compiles the java classes in java.desktop (and its dependencies). Works for any module name (and -gensrc, -libs, -launchers, -gendata, -copy) In addition to this, the suffix -only can be added to any target to disable the prerequisites for it. Using this is not recommended but it may save time when doing certain incremental builds and you are in a terrible hurry. For incremental builds, sjavac can be used and works reasonably well (configure --enable-sjavac). Work is in progress on making it work even better. The old workaround JDK_FILTER=package/to/compile is still working. The clean target is still oriented around repositories, mostly because the build output is still in large parts repository oriented. This is something we hope to improve later. /Erik On 2014-08-12 16:10, Chris Hegarty wrote: This is a review request for the Initial changes for JEP 201: Modular Source Code [1]. There are a number of individuals responsible for these changes. Some, possibly not all, are explicitly listed in the To section of this mail, and they will help address any comments arising from this review request. For the purposes of review, the actual source file moves have been omitted from the webrev below, with the exception of any source file that has a change to it’s actual content. The new location of the source files can be determined from JEP 201 [1] and JEP 200 "The Modular JDK" [2], or by browsing the staging forest [3]. Webrevs: http://cr.openjdk.java.net/~chegar/8054834/00/ Due to the significant impact of these changes, a JDK 9 promotion has been tentatively reserved for their integration. All comments are welcome, although given the nature of the changes then we might have to create separate issues in JIRA to address some of them later in jdk9/dev.. -Chris. [1] https://bugs.openjdk.java.net/browse/JDK-8051619 [2] https://bugs.openjdk.java.net/browse/JDK-8051618 [3] http://hg.openjdk.java.net/jigsaw/stage
Re: RFR [9] Modular Source Code
On 2014-08-13 14:17, Chris Hegarty wrote: Thanks for the explanation Erik. I have taken a pass over the changes, and they look ok to me ( I am happy to be listed as a reviewer ). I also did several build and test runs on Solaris, Linux, Max OSX, and Windows. All look good. I am seeing, in some cases, about a 20% reduction in image build times on an 8 core i7, running Linux x86. Nice to hear! One question; Are there any new requirements on build systems as a result of these changes? Yes and no. The build should still be compatible with gnu make 3.81. However, certain builds of that make version (particularly the one we have used for jdk8 internally) are known to crash in Cygwin and it is instead recommended to use a newer 4.0 version of make in Cygwin. Also to get good concurrent build performance on Windows, gnu make 4.0 is required for Cygwin. I have done test builds with msys and it seems to be working too. One more thing that I forgot to mention. If you are using gnu make 4.0, there is a new feature called output-sync that can be enabled that will make it somewhat easier to read the build output. This can be enabled either with the configure parameter --with-output-sync=recurse, or at the make command line "make OUTPUT_SYNC=recurse". (it will be enabled by default in JPRT). More information on this can be found here: http://www.gnu.org/software/make/manual/make.html#Parallel-Output /Erik
Re: RFR [9] Modular Source Code
Just to add to Chris and Erik's mails, I would encourage everyone that pushes to jdk9/dev or the other jdk9 project integration forests to clone and build jigsaw/stage and get familiar with the proposed layout, new build targets, and the very different output emitted during the build. The changes are arguably as significant as the transition in JDK 8 from the "old build" to the "new build" so the more people taking the forest for a test drive the better. If you maintain your our own own IDE project then you'll likely have to adjust file paths so any issues encountered would be useful to hear about too. -Alan.
Re: RFR [9] Modular Source Code
I should probably write something about the rather extensive changes to the build logic in this patch. As the source gets organized around modules, it made sense to also organize the build more around modules. In this patch, the makefiles have in large parts been split into module specific files and the top level targets are oriented around modules. This means the top level targets are much more fine granular than currently in JDK 9. Another difference is that the top level targets are now able to run concurrently, to give more opportunity for utilizing multiple cpus. In JDK 9, the build moves sequentially through each repository, in a given order, now make is free to run more things in parallel. This makes the build faster, at least on beefy hardware. The drawback is that the build log gets a bit more confusing. When something fails, it won't interrupt other building threads at once, so the actual failure may be further up the log (even further than before). I have found that searching for "Error 2" is a good way to find the real failure. Another consequence is that the build time summary at the end only displays total time as there is no good definition for how much time was spent in each repository anymore. A summary on the new targets: make [default] Does pretty much the same as before. It compiles everything but does not build all jars or create images. make all Builds everything, including jars, images and docs. Also runs a verification tool on the java classes which will point out any broken module boundaries. make images Same as before make hotspot Builds the hotspot repository, like before. make docs Builds all the documentation, including javadoc. make docs-javadoc Builds just the javadoc. This target has very few prerequisites so provides a fast way to just build javadoc. make gensrc Runs all source code generation steps. make java Compiles all java classes. Other similar targets are libs, launchers, gendata and copy make java.desktop Compiles everything in the java.desktop module (and its dependencies), both java and native code. Works for any module name. make java.desktop-java Compiles the java classes in java.desktop (and its dependencies). Works for any module name (and -gensrc, -libs, -launchers, -gendata, -copy) In addition to this, the suffix -only can be added to any target to disable the prerequisites for it. Using this is not recommended but it may save time when doing certain incremental builds and you are in a terrible hurry. For incremental builds, sjavac can be used and works reasonably well (configure --enable-sjavac). Work is in progress on making it work even better. The old workaround JDK_FILTER=package/to/compile is still working. The clean target is still oriented around repositories, mostly because the build output is still in large parts repository oriented. This is something we hope to improve later. /Erik On 2014-08-12 16:10, Chris Hegarty wrote: This is a review request for the Initial changes for JEP 201: Modular Source Code [1]. There are a number of individuals responsible for these changes. Some, possibly not all, are explicitly listed in the To section of this mail, and they will help address any comments arising from this review request. For the purposes of review, the actual source file moves have been omitted from the webrev below, with the exception of any source file that has a change to it’s actual content. The new location of the source files can be determined from JEP 201 [1] and JEP 200 "The Modular JDK" [2], or by browsing the staging forest [3]. Webrevs: http://cr.openjdk.java.net/~chegar/8054834/00/ Due to the significant impact of these changes, a JDK 9 promotion has been tentatively reserved for their integration. All comments are welcome, although given the nature of the changes then we might have to create separate issues in JIRA to address some of them later in jdk9/dev.. -Chris. [1] https://bugs.openjdk.java.net/browse/JDK-8051619 [2] https://bugs.openjdk.java.net/browse/JDK-8051618 [3] http://hg.openjdk.java.net/jigsaw/stage