Re: RFR: 8256354: Github Action build on Windows should define OS and MSVC versions
On Fri, 13 Nov 2020 17:26:28 GMT, Robin Westberg wrote: > We should be more explicit about OS and compiler versions used in the GitHub > Actions builds, to avoid problems caused by unexpected changes to the > defaults. This patch changes the OS and MSVC versions used from latest > (currently 2019) / default (currently 14.28) to 2019 / 14.27. Looks good to me. I wondered if https://github.com/marketplace/actions/setup-msbuild could have been used instead, but this is fine as well. - Marked as reviewed by shade (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/1207
Re: RFR: 8256354: Github Action build on Windows should define OS and MSVC versions
On Fri, 13 Nov 2020 17:26:28 GMT, Robin Westberg wrote: > We should be more explicit about OS and compiler versions used in the GitHub > Actions builds, to avoid problems caused by unexpected changes to the > defaults. This patch changes the OS and MSVC versions used from latest > (currently 2019) / default (currently 14.28) to 2019 / 14.27. Marked as reviewed by erikj (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/1207
RFR: 8256354: Github Action build on Windows should define OS and MSVC versions
We should be more explicit about OS and compiler versions used in the GitHub Actions builds, to avoid problems caused by unexpected changes to the defaults. This patch changes the OS and MSVC versions used from latest (currently 2019) / default (currently 14.28) to 2019 / 14.27. - Commit messages: - Use Windows 2019 and MSVC 14.27 Changes: https://git.openjdk.java.net/jdk/pull/1207/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=1207=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8256354 Stats: 9 lines in 1 file changed: 7 ins; 0 del; 2 mod Patch: https://git.openjdk.java.net/jdk/pull/1207.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/1207/head:pull/1207 PR: https://git.openjdk.java.net/jdk/pull/1207
Re: RFR: 8256308: Send arguments to javac server in a config file
On Thu, 12 Nov 2020 22:13:37 GMT, Magnus Ihse Bursie wrote: > Currently, to use the javac server, a horrendously long command line option > is created, looking like this: `--server:portfile= portfile>:sjavac=`, where the sjavac command has > had all spaces replaced by %20. Since Project Jigsaw, the set of module > arguments needed is huge to begin with, making this command line > incomprehensible after mangling. > > Apart from making java command lines hard to read (and copy/paste!) by > developers, it also makes it hard for scripts to parse. The upcoming winenv > rewrite is dependent on being able to differentiate between path names and > other arguments, which is not possible in this mess. > > So, instead, let's write it to a file, without any escaping, and just pass > the configuration file name to the server. > > Note that this will change the behavior of the javac server, but as the > source code states this is not a documented or externally supported API no > CSR is needed. > > I also cleaned up some code in SjavacClient, in particular code relating to > the passing of arguments. (We never change poolsize or keepalive when we call > it.) Looks good in general, just some whitespace issues. - PR: https://git.openjdk.java.net/jdk/pull/1195
Re: RFR: 8256308: Send arguments to javac server in a config file [v5]
On Fri, 13 Nov 2020 01:08:09 GMT, Magnus Ihse Bursie wrote: >> Currently, to use the javac server, a horrendously long command line option >> is created, looking like this: `--server:portfile=> portfile>:sjavac=`, where the sjavac command has >> had all spaces replaced by %20. Since Project Jigsaw, the set of module >> arguments needed is huge to begin with, making this command line >> incomprehensible after mangling. >> >> Apart from making java command lines hard to read (and copy/paste!) by >> developers, it also makes it hard for scripts to parse. The upcoming winenv >> rewrite is dependent on being able to differentiate between path names and >> other arguments, which is not possible in this mess. >> >> So, instead, let's write it to a file, without any escaping, and just pass >> the configuration file name to the server. >> >> Note that this will change the behavior of the javac server, but as the >> source code states this is not a documented or externally supported API no >> CSR is needed. >> >> I also cleaned up some code in SjavacClient, in particular code relating to >> the passing of arguments. (We never change poolsize or keepalive when we >> call it.) > > Magnus Ihse Bursie has updated the pull request incrementally with one > additional commit since the last revision: > > MODULE_SUBDIR must be defined before using make/common/JavaCompilation.gmk line 237: > 235: $1_JAVAC_SERVER_CMD := $$(JAVA_DETACH) $$($1_JAVA_FLAGS) > $$($1_JAVAC) > 236: > 237: $1_CONFIG_VARDEPS :=$$($1_JAVAC_PORT_FILE) $$($1_JAVAC_SERVER_CMD) Space make/common/JavaCompilation.gmk line 239: > 237: $1_CONFIG_VARDEPS :=$$($1_JAVAC_PORT_FILE) $$($1_JAVAC_SERVER_CMD) > 238: $1_CONFIG_VARDEPS_FILE := $$(call DependOnVariable, > $1_CONFIG_VARDEPS, \ > 239: $$($1_BIN)$$($1_MODULE_SUBDIR)/_the.$1.config_vardeps) Indentation 4 make/common/JavaCompilation.gmk line 246: > 244: $1_ECHO_COMMAND := $(ECHO) > 245: endif > 246: $$($1_JAVAC_SERVER_CONFIG): $$($1_CONFIG_VARDEPS_FILE) Indentation? - PR: https://git.openjdk.java.net/jdk/pull/1195
Re: RFR: 8256277: Github Action build on macOS should define OS and Xcode versions
On Fri, 13 Nov 2020 11:48:31 GMT, Robin Westberg wrote: > We should be more explicit about OS and compiler versions used in the GitHub > Actions builds, to avoid problems caused by unexpected changes to the > defaults. This patch changes the OS and Xcode versions used from latest > (currently 10.15) / default (currently 12.0) to 10.15 / 11.3.1. Marked as reviewed by erikj (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/1201
Integrated: 8256127: Add cross-compiled foreign architectures builds to submit workflow
On Tue, 10 Nov 2020 19:05:31 GMT, Aleksey Shipilev wrote: > It is > [possible](https://github.com/openjdk/jdk/blob/master/doc/building.md#creating-and-using-sysroots-with-qemu-deboostrap) > to efficiently cross-compile to foreign architectures on current GH actions > that are driven by Ubuntu. I have been using this method for years to produce > binaries at [builds.shipilev.net](https://builds.shipilev.net). > > These cross-compilation targets frequently find arch-specific build bugs. > Hotspot is rather well insulated from the fundamental problems in > arch-support, so the overwhelming majority of those arch-specific bugs are > simple omissions of `#include`-s, inconsistent `#ifdef`-ing, missed method > renames / signature changes, or incorrect removals of methods that are used > by arch-specific code. Those are straightforward to fix, if the contributor > knows about them. My experience says the common attitude to these fixes is: > "Oh, I would have fixed it in the original change if I knew about this." > > This improvement adds several foreign architectures to GH actions workflow to > provide the automatic safety net to give early warning for these cases. In > fact, many Hotspot contributors already build AArch64, ARM, PPC64 targets as > pre-integration sanity checks, and this does the important checks > automatically for them. > > To optimize workflow costs, the change does the following: > - The build is only done for hotspot-debug-no-pch, as the most frequent > place where arch-specific build bugs show up. > - There are no tests, and in fact the arch-specific binary does not even > run. The build is purely about the build. > - Foreign architectures reuse the Linux x86_64 release build as build JDK. > This avoids building "host" build JDK as it would otherwise happen with > cross-compiling. > - The created sysroot is cached and keyed on `submit.yml` hash. So it would > regenerate only if the workflow itself changes. This saves about 10 minutes > per arch. > > Space-wise, sysroots in GH cache take about 580M uncompressed, and 270M > zstd-compressed bundle. In the end, this adds 4x270 = 1080M into local cache. > > Time-wise, ball-parking the current workflow budget [looks like > this](https://github.com/shipilev/jdk/runs/1389151071): > - Linux x86_64: >- builds: 120m >- tests: 200m > - Linux x86_32: >- builds: 75m >- tests: 235m > - Windows x86_64 >- builds: 120m >- tests: 290m > - MacOS x86_64 >- builds: 75m >- tests: 165m > - Linux aarch64: >- builds: 25m (+10m to create uncached sysroot) > - Linux arm: >- builds: 20m (+10m to create uncached sysroot) > - Linux ppc64le: >- builds: 20m (+15m to create uncached sysroot) > - Linux s390x: >- builds: 20m (+10m to create uncached sysroot) > > In other words, new workflow takes about 715 Linux-host-minutes, 410 > Windows-host-minutes, 240 Mac-host-minutes. Out of which new jobs take about > 85 Linux-host-minutes. So the cost of new jobs is roughly: > - 11.9% of Linux-host-minutes > - 6.2% of all-host-minutes > - 2.2% of runner-minutes, if you [weigh in the proportional cost of > non-Linux > runners](https://docs.github.com/en/free-pro-team@latest/github/setting-up-and-managing-billing-and-payments-on-github/about-billing-for-github-actions) > > In other words, the additional runner costs are pale in comparison with what > is already done in build and test jobs. This pull request has now been integrated. Changeset: e9956fec Author:Aleksey Shipilev URL: https://git.openjdk.java.net/jdk/commit/e9956fec Stats: 557 lines in 1 file changed: 556 ins; 0 del; 1 mod 8256127: Add cross-compiled foreign architectures builds to submit workflow Reviewed-by: ihse, rwestberg - PR: https://git.openjdk.java.net/jdk/pull/1147
Re: RFR: 8256127: Add cross-compiled foreign architectures builds to submit workflow [v6]
On Fri, 13 Nov 2020 11:14:42 GMT, Aleksey Shipilev wrote: >> Yeah, it would certainly need a bit of adaptation to capture the differences >> properly, so perfectly fine to look into later! > > Thanks folks! I'll merge the master and see if it is still green, and if it > is, I'll integrate. Cross-compiled builds still look green. Integrating. - PR: https://git.openjdk.java.net/jdk/pull/1147
Re: RFR: 8256277: Github Action build on macOS should define OS and Xcode versions
On Fri, 13 Nov 2020 11:58:19 GMT, Aleksey Shipilev wrote: >> We should be more explicit about OS and compiler versions used in the GitHub >> Actions builds, to avoid problems caused by unexpected changes to the >> defaults. This patch changes the OS and Xcode versions used from latest >> (currently 10.15) / default (currently 12.0) to 10.15 / 11.3.1. > > Looks fine to me. I was wondering if we should do the same for > `ubuntu-latest` and `windows-latest`. Thanks for reviewing! Yeah, planning to do something similar for the other platforms as well! - PR: https://git.openjdk.java.net/jdk/pull/1201
Re: RFR: 8256277: Github Action build on macOS should define OS and Xcode versions
On Fri, 13 Nov 2020 11:48:31 GMT, Robin Westberg wrote: > We should be more explicit about OS and compiler versions used in the GitHub > Actions builds, to avoid problems caused by unexpected changes to the > defaults. This patch changes the OS and Xcode versions used from latest > (currently 10.15) / default (currently 12.0) to 10.15 / 11.3.1. Looks good! - Marked as reviewed by ehelin (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/1201
Re: RFR: 8256277: Github Action build on macOS should define OS and Xcode versions
On Fri, 13 Nov 2020 11:48:31 GMT, Robin Westberg wrote: > We should be more explicit about OS and compiler versions used in the GitHub > Actions builds, to avoid problems caused by unexpected changes to the > defaults. This patch changes the OS and Xcode versions used from latest > (currently 10.15) / default (currently 12.0) to 10.15 / 11.3.1. Looks fine to me. I was wondering if we should do the same for `ubuntu-latest` and `windows-latest`. - Marked as reviewed by shade (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/1201
RFR: 8256277: Github Action build on macOS should define OS and Xcode versions
We should be more explicit about OS and compiler versions used in the GitHub Actions builds, to avoid problems caused by unexpected changes to the defaults. This patch changes the OS and Xcode versions used from latest (currently 10.15) / default (currently 12.0) to 10.15 / 11.3.1. - Commit messages: - Select explicit versions of macOS and Xcode Changes: https://git.openjdk.java.net/jdk/pull/1201/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=1201=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8256277 Stats: 8 lines in 1 file changed: 6 ins; 0 del; 2 mod Patch: https://git.openjdk.java.net/jdk/pull/1201.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/1201/head:pull/1201 PR: https://git.openjdk.java.net/jdk/pull/1201
Re: RFR: 8256127: Add cross-compiled foreign architectures builds to submit workflow [v7]
> It is > [possible](https://github.com/openjdk/jdk/blob/master/doc/building.md#creating-and-using-sysroots-with-qemu-deboostrap) > to efficiently cross-compile to foreign architectures on current GH actions > that are driven by Ubuntu. I have been using this method for years to produce > binaries at [builds.shipilev.net](https://builds.shipilev.net). > > These cross-compilation targets frequently find arch-specific build bugs. > Hotspot is rather well insulated from the fundamental problems in > arch-support, so the overwhelming majority of those arch-specific bugs are > simple omissions of `#include`-s, inconsistent `#ifdef`-ing, missed method > renames / signature changes, or incorrect removals of methods that are used > by arch-specific code. Those are straightforward to fix, if the contributor > knows about them. My experience says the common attitude to these fixes is: > "Oh, I would have fixed it in the original change if I knew about this." > > This improvement adds several foreign architectures to GH actions workflow to > provide the automatic safety net to give early warning for these cases. In > fact, many Hotspot contributors already build AArch64, ARM, PPC64 targets as > pre-integration sanity checks, and this does the important checks > automatically for them. > > To optimize workflow costs, the change does the following: > - The build is only done for hotspot-debug-no-pch, as the most frequent > place where arch-specific build bugs show up. > - There are no tests, and in fact the arch-specific binary does not even > run. The build is purely about the build. > - Foreign architectures reuse the Linux x86_64 release build as build JDK. > This avoids building "host" build JDK as it would otherwise happen with > cross-compiling. > - The created sysroot is cached and keyed on `submit.yml` hash. So it would > regenerate only if the workflow itself changes. This saves about 10 minutes > per arch. > > Space-wise, sysroots in GH cache take about 580M uncompressed, and 270M > zstd-compressed bundle. In the end, this adds 4x270 = 1080M into local cache. > > Time-wise, ball-parking the current workflow budget [looks like > this](https://github.com/shipilev/jdk/runs/1389151071): > - Linux x86_64: >- builds: 120m >- tests: 200m > - Linux x86_32: >- builds: 75m >- tests: 235m > - Windows x86_64 >- builds: 120m >- tests: 290m > - MacOS x86_64 >- builds: 75m >- tests: 165m > - Linux aarch64: >- builds: 25m (+10m to create uncached sysroot) > - Linux arm: >- builds: 20m (+10m to create uncached sysroot) > - Linux ppc64le: >- builds: 20m (+15m to create uncached sysroot) > - Linux s390x: >- builds: 20m (+10m to create uncached sysroot) > > In other words, new workflow takes about 715 Linux-host-minutes, 410 > Windows-host-minutes, 240 Mac-host-minutes. Out of which new jobs take about > 85 Linux-host-minutes. So the cost of new jobs is roughly: > - 11.9% of Linux-host-minutes > - 6.2% of all-host-minutes > - 2.2% of runner-minutes, if you [weigh in the proportional cost of > non-Linux > runners](https://docs.github.com/en/free-pro-team@latest/github/setting-up-and-managing-billing-and-payments-on-github/about-billing-for-github-actions) > > In other words, the additional runner costs are pale in comparison with what > is already done in build and test jobs. Aleksey Shipilev has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains eight additional commits since the last revision: - Merge branch 'master' into JDK-8256127-foreign-build - Build only hotspot reduced build - Merge branch 'master' into JDK-8256127-foreign-build - Merge branch 'master' into JDK-8256127-foreign-build - Incomplete fix: remove special dirs - Merge branch 'master' into JDK-8256127-foreign-build - Remove special directories from chroot to make it archivable - 8256127: Add cross-compiled foreign architectures builds to submit workflow - Changes: - all: https://git.openjdk.java.net/jdk/pull/1147/files - new: https://git.openjdk.java.net/jdk/pull/1147/files/81e07fe2..b0dfab5f Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=1147=06 - incr: https://webrevs.openjdk.java.net/?repo=jdk=1147=05-06 Stats: 13479 lines in 272 files changed: 7889 ins; 3996 del; 1594 mod Patch: https://git.openjdk.java.net/jdk/pull/1147.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/1147/head:pull/1147 PR: https://git.openjdk.java.net/jdk/pull/1147
Re: RFR: 8256127: Add cross-compiled foreign architectures builds to submit workflow [v6]
On Fri, 13 Nov 2020 11:04:38 GMT, Robin Westberg wrote: >>> Looks good! I did a similar attempt at cross building a while ago, but >>> never got around to finishing it, so it's nice to see it materializing! I >>> do have a general comment on reducing the amount of duplicated content >>> though. Since all these cross-build platforms share the same prerequisites, >>> they can be expressed as matrix builds. Here's what I did: >>> https://github.com/rwestberg/jdk/blob/947c934621c3013c055152356615e0120382cedf/.github/workflows/submit.yml#L102 >> >> Right. AFAIU your code, it bootstraps the chroot and builds x86_64 build JDK >> for every config and every run, something this PR is able to avoid. We'd >> need to figure that out. I think that is pretty doable, but it would require >> a few days worth of pipeline testing to work out the kinks. So, would you >> mind we do that in the follow-ups? > > Yeah, it would certainly need a bit of adaptation to capture the differences > properly, so perfectly fine to look into later! Thanks folks! I'll merge the master and see if it is still green, and if it is, I'll integrate. - PR: https://git.openjdk.java.net/jdk/pull/1147
Re: RFR: 8256127: Add cross-compiled foreign architectures builds to submit workflow [v6]
On Fri, 13 Nov 2020 10:54:01 GMT, Aleksey Shipilev wrote: >> Looks good! I did a similar attempt at cross building a while ago, but never >> got around to finishing it, so it's nice to see it materializing! I do have >> a general comment on reducing the amount of duplicated content though. Since >> all these cross-build platforms share the same prerequisites, they can be >> expressed as matrix builds. Here's what I did: >> https://github.com/rwestberg/jdk/blob/947c934621c3013c055152356615e0120382cedf/.github/workflows/submit.yml#L102 >> >> You'd have to adjust the details obviously, but I think it could help with >> future maintainability. >> >> Another minor comment is that it may be faster to use >> http://debian-archive.trafficmanager.net/debian/ instead of >> http://httpredir.debian.org/debian/ (the former is Azure-specific but I >> don't think it's part of the list that the latter uses). But since it will >> be cached after first use it probably doesn't matter much. > >> Looks good! I did a similar attempt at cross building a while ago, but never >> got around to finishing it, so it's nice to see it materializing! I do have >> a general comment on reducing the amount of duplicated content though. Since >> all these cross-build platforms share the same prerequisites, they can be >> expressed as matrix builds. Here's what I did: >> https://github.com/rwestberg/jdk/blob/947c934621c3013c055152356615e0120382cedf/.github/workflows/submit.yml#L102 > > Right. AFAIU your code, it bootstraps the chroot and builds x86_64 build JDK > for every config and every run, something this PR is able to avoid. We'd need > to figure that out. I think that is pretty doable, but it would require a few > days worth of pipeline testing to work out the kinks. So, would you mind we > do that in the follow-ups? Yeah, it would certainly need a bit of adaptation to capture the differences properly, so perfectly fine to look into later! - PR: https://git.openjdk.java.net/jdk/pull/1147
Re: RFR: 8256127: Add cross-compiled foreign architectures builds to submit workflow [v6]
On Fri, 13 Nov 2020 10:07:26 GMT, Robin Westberg wrote: > Looks good! I did a similar attempt at cross building a while ago, but never > got around to finishing it, so it's nice to see it materializing! I do have a > general comment on reducing the amount of duplicated content though. Since > all these cross-build platforms share the same prerequisites, they can be > expressed as matrix builds. Here's what I did: > https://github.com/rwestberg/jdk/blob/947c934621c3013c055152356615e0120382cedf/.github/workflows/submit.yml#L102 Right. AFAIU your code, it bootstraps the chroot and builds x86_64 build JDK for every config and every run, something this PR is able to avoid. We'd need to figure that out. I think that is pretty doable, but it would require a few days worth of pipeline testing to work out the kinks. So, would you mind we do that in the follow-ups? - PR: https://git.openjdk.java.net/jdk/pull/1147
Re: RFR: 8256127: Add cross-compiled foreign architectures builds to submit workflow [v6]
On Thu, 12 Nov 2020 06:50:08 GMT, Aleksey Shipilev wrote: >> It is >> [possible](https://github.com/openjdk/jdk/blob/master/doc/building.md#creating-and-using-sysroots-with-qemu-deboostrap) >> to efficiently cross-compile to foreign architectures on current GH actions >> that are driven by Ubuntu. I have been using this method for years to >> produce binaries at [builds.shipilev.net](https://builds.shipilev.net). >> >> These cross-compilation targets frequently find arch-specific build bugs. >> Hotspot is rather well insulated from the fundamental problems in >> arch-support, so the overwhelming majority of those arch-specific bugs are >> simple omissions of `#include`-s, inconsistent `#ifdef`-ing, missed method >> renames / signature changes, or incorrect removals of methods that are used >> by arch-specific code. Those are straightforward to fix, if the contributor >> knows about them. My experience says the common attitude to these fixes is: >> "Oh, I would have fixed it in the original change if I knew about this." >> >> This improvement adds several foreign architectures to GH actions workflow >> to provide the automatic safety net to give early warning for these cases. >> In fact, many Hotspot contributors already build AArch64, ARM, PPC64 targets >> as pre-integration sanity checks, and this does the important checks >> automatically for them. >> >> To optimize workflow costs, the change does the following: >> - The build is only done for hotspot-debug-no-pch, as the most frequent >> place where arch-specific build bugs show up. >> - There are no tests, and in fact the arch-specific binary does not even >> run. The build is purely about the build. >> - Foreign architectures reuse the Linux x86_64 release build as build JDK. >> This avoids building "host" build JDK as it would otherwise happen with >> cross-compiling. >> - The created sysroot is cached and keyed on `submit.yml` hash. So it >> would regenerate only if the workflow itself changes. This saves about 10 >> minutes per arch. >> >> Space-wise, sysroots in GH cache take about 580M uncompressed, and 270M >> zstd-compressed bundle. In the end, this adds 4x270 = 1080M into local cache. >> >> Time-wise, ball-parking the current workflow budget [looks like >> this](https://github.com/shipilev/jdk/runs/1389151071): >> - Linux x86_64: >>- builds: 120m >>- tests: 200m >> - Linux x86_32: >>- builds: 75m >>- tests: 235m >> - Windows x86_64 >>- builds: 120m >>- tests: 290m >> - MacOS x86_64 >>- builds: 75m >>- tests: 165m >> - Linux aarch64: >>- builds: 25m (+10m to create uncached sysroot) >> - Linux arm: >>- builds: 20m (+10m to create uncached sysroot) >> - Linux ppc64le: >>- builds: 20m (+15m to create uncached sysroot) >> - Linux s390x: >>- builds: 20m (+10m to create uncached sysroot) >> >> In other words, new workflow takes about 715 Linux-host-minutes, 410 >> Windows-host-minutes, 240 Mac-host-minutes. Out of which new jobs take about >> 85 Linux-host-minutes. So the cost of new jobs is roughly: >> - 11.9% of Linux-host-minutes >> - 6.2% of all-host-minutes >> - 2.2% of runner-minutes, if you [weigh in the proportional cost of >> non-Linux >> runners](https://docs.github.com/en/free-pro-team@latest/github/setting-up-and-managing-billing-and-payments-on-github/about-billing-for-github-actions) >> >> In other words, the additional runner costs are pale in comparison with what >> is already done in build and test jobs. > > Aleksey Shipilev has updated the pull request incrementally with one > additional commit since the last revision: > > Build only hotspot reduced build Looks good! I did a similar attempt at cross building a while ago, but never got around to finishing it, so it's nice to see it materializing! I do have a general comment on reducing the amount of duplicated content though. Since all these cross-build platforms share the same prerequisites, they can be expressed as matrix builds. Here's what I did: https://github.com/rwestberg/jdk/blob/947c934621c3013c055152356615e0120382cedf/.github/workflows/submit.yml#L102 You'd have to adjust the details obviously, but I think it could help with future maintainability. Another minor comment is that it may be faster to use http://debian-archive.trafficmanager.net/debian/ instead of http://httpredir.debian.org/debian/ (the former is Azure-specific but I don't think it's part of the list that the latter uses). But since it will be cached after first use it probably doesn't matter much. - Marked as reviewed by rwestberg (Committer). PR: https://git.openjdk.java.net/jdk/pull/1147
Re: RFR: 8256127: Add cross-compiled foreign architectures builds to submit workflow [v5]
On Thu, 12 Nov 2020 10:08:39 GMT, Volker Simonis wrote: >>> I'm not at all sure I want to accept these changes. I understand that you >>> care about these exotic platforms, and so do I, but adding them to the >>> submit workflow for _all_ changes seem like the wrong way to proceed. >> >> I understand the first reaction, but please read the updated description. It >> includes the discussion what bugs it tries to catch, and updated cost >> estimates. >> >> Submit workflow is a convenience thing for better testing coverage. Having >> the early warnings for fixable issues in foreign architectures helps to >> avoid follow-up churn without increasing the costs all too much. Do note >> that it does not require contributors to fix the runtime issues in foreign >> architectures, it only asks for passing a rather low bar of _not breaking >> the builds_ for them, which is, in my experience, an easily achievable goal. >> It is easily achievable because -- I am speaking from experience of fixing >> a lot of them over the years! -- the overwhelming majority of arch-specific >> build breakages are about silly things that are obvious from the build logs. >> >> So I am leaving this PR open, letting others to chime in. If there is really >> a need for a wider discussion here, I could start a thread at jdk-dev@, but >> so far it does not look as a good use of our collective time. Please read >> through the description, and see if that soothes your concerns. >> >> Aside: the issues about costs were [raised >> before](https://mail.openjdk.java.net/pipermail/skara-dev/2020-October/003581.html), >> but I think all of them resolve as "enjoy your free tier". If we target to >> optimize the GH action costs, IMO we should instead consider to avoid >> triggering them on every push, as [I hinted >> before](https://mail.openjdk.java.net/pipermail/jdk-dev/2020-September/004739.html), >> and which I think requires better Skara integration (i.e. for `/test` >> command). Meanwhile, this PR gives us extended build coverage at fraction of >> the additional cost. > > I think this is a really useful feature and I'm a little surprised that it > gets rejected. Please find my comments inline. > >> Aleksey, >> >> I'm not at all sure I want to accept these changes. I understand that you >> care about these exotic platforms, and so do I, but adding them to the >> submit workflow for _all_ changes seem like the wrong way to proceed. >> >> My concern vary all the way from increased build time, to increased usage of >> the Github CPU quota (which, as I understand it, is fairly large but not >> unlimited), to the elephant in the room: the question on who the burden lays >> to fix bugs in these exotic platforms. It is not at all obvious that these >> builds should be tested before commit. >> > > I think "on who's the burden to fix issues" on *exotic* platforms and > "testing changes on all platforms" at commit time are two totally distinct > questions. While I agree that problems on a *exotic* platform shouldn't block > reviewed changes for an unreasonable amount of time, I also think it is very > valuable for both, authors of changes and maintainers of *exotic* platforms > to at least get early notifications of problems on other platforms. > > My experience as a maintainer of the AIX7PowerPC/s390 ports has always been > very positive with regards to the collaboration with authors of changes which > impact these ports. Authors are anxious to not break other platforms but > usually don't have the possibility to test on these platforms. Aleksey's > change comes in very handy here. It will help authors to fix trivial build > problems right in their initial submission which I'm sure they'll appreciate. > For more complex problems it gives Authors the possibility to notify port > maintainers early, before a change gets pushed. I'm sure this alerting could > even be automatized in the future if builds/tests fail on certain platforms. > > Of course such problems shouldn't block changes which are otherwise reviewed > and ready to push for an unreasonable amount of time, so I have nothing > against making the test on such platform optional submit requirements. > > But overall I think Aleksey's enhancement are a great means to improve the > development experience and to keep the OpenJDK repos clean of trivial > follow-up fixes. > > > PS: I specially emphasized *exotic platforms* because it seems to me that > your comment implies that all not Oracle-supported platforms are considered > *exotic* for you. I think in an open source project it should be decided by > the community which platforms are considered *main* (or *primary*) and which > are considered *secondary* platforms. > >> I think this change warrants a much wider and deeper discussion about the >> purpose of the Github commit hook actions, how the free Github CPU quota >> should be spent, and on whom the responsibility lays to
Re: RFR: 8256127: Add cross-compiled foreign architectures builds to submit workflow [v6]
On Thu, 12 Nov 2020 06:50:08 GMT, Aleksey Shipilev wrote: >> It is >> [possible](https://github.com/openjdk/jdk/blob/master/doc/building.md#creating-and-using-sysroots-with-qemu-deboostrap) >> to efficiently cross-compile to foreign architectures on current GH actions >> that are driven by Ubuntu. I have been using this method for years to >> produce binaries at [builds.shipilev.net](https://builds.shipilev.net). >> >> These cross-compilation targets frequently find arch-specific build bugs. >> Hotspot is rather well insulated from the fundamental problems in >> arch-support, so the overwhelming majority of those arch-specific bugs are >> simple omissions of `#include`-s, inconsistent `#ifdef`-ing, missed method >> renames / signature changes, or incorrect removals of methods that are used >> by arch-specific code. Those are straightforward to fix, if the contributor >> knows about them. My experience says the common attitude to these fixes is: >> "Oh, I would have fixed it in the original change if I knew about this." >> >> This improvement adds several foreign architectures to GH actions workflow >> to provide the automatic safety net to give early warning for these cases. >> In fact, many Hotspot contributors already build AArch64, ARM, PPC64 targets >> as pre-integration sanity checks, and this does the important checks >> automatically for them. >> >> To optimize workflow costs, the change does the following: >> - The build is only done for hotspot-debug-no-pch, as the most frequent >> place where arch-specific build bugs show up. >> - There are no tests, and in fact the arch-specific binary does not even >> run. The build is purely about the build. >> - Foreign architectures reuse the Linux x86_64 release build as build JDK. >> This avoids building "host" build JDK as it would otherwise happen with >> cross-compiling. >> - The created sysroot is cached and keyed on `submit.yml` hash. So it >> would regenerate only if the workflow itself changes. This saves about 10 >> minutes per arch. >> >> Space-wise, sysroots in GH cache take about 580M uncompressed, and 270M >> zstd-compressed bundle. In the end, this adds 4x270 = 1080M into local cache. >> >> Time-wise, ball-parking the current workflow budget [looks like >> this](https://github.com/shipilev/jdk/runs/1389151071): >> - Linux x86_64: >>- builds: 120m >>- tests: 200m >> - Linux x86_32: >>- builds: 75m >>- tests: 235m >> - Windows x86_64 >>- builds: 120m >>- tests: 290m >> - MacOS x86_64 >>- builds: 75m >>- tests: 165m >> - Linux aarch64: >>- builds: 25m (+10m to create uncached sysroot) >> - Linux arm: >>- builds: 20m (+10m to create uncached sysroot) >> - Linux ppc64le: >>- builds: 20m (+15m to create uncached sysroot) >> - Linux s390x: >>- builds: 20m (+10m to create uncached sysroot) >> >> In other words, new workflow takes about 715 Linux-host-minutes, 410 >> Windows-host-minutes, 240 Mac-host-minutes. Out of which new jobs take about >> 85 Linux-host-minutes. So the cost of new jobs is roughly: >> - 11.9% of Linux-host-minutes >> - 6.2% of all-host-minutes >> - 2.2% of runner-minutes, if you [weigh in the proportional cost of >> non-Linux >> runners](https://docs.github.com/en/free-pro-team@latest/github/setting-up-and-managing-billing-and-payments-on-github/about-billing-for-github-actions) >> >> In other words, the additional runner costs are pale in comparison with what >> is already done in build and test jobs. > > Aleksey Shipilev has updated the pull request incrementally with one > additional commit since the last revision: > > Build only hotspot reduced build Marked as reviewed by ihse (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/1147
