Bug#987658: unblock: openjdk-11-jre-dcevm/11.0.11+9-1
Le 2021-05-19 20:58, Paul Gevers a écrit : unblocked. Thank you very much Paul!
Bug#987658: unblock: openjdk-11-jre-dcevm/11.0.11+9-1
On 15/05/2021 07:51, Paul Gevers wrote: > Are commits f8bc235 and 10263b1 the head of the OpenJDK archive? Yes, but these commits refer to the AdoptOpenJDK Git mirror [1] of the upstream Mercurial repository [2]. src:openjdk-11 pulls directly from Mercurial but the source tree is identical. > Does OpenJDK in Debian carry any patches you want to have too? Yes, DCEVM picks a patch from openjdk-11 to support multi arch libraries [3]. The other patches are not relevant to DCEVM. > I start to realize that src:openjdk-11 is probably more than the source > of src:openjdk-11-jre-dcevm, as I *assume* that the source of the latter > is only a *subset* of the source of the former. What would we need to do > to compare those sources? Can you indeed confirm that the delta between > both sources is exactly the patch set? The source is the same, but DCEVM builds only the HotSpot VM, not the Java standard library nor the developer tools. DCEVM just swaps the JVM at runtime, everything else is provided by openjdk-11, that's why the two packages are tightly coupled. I can confirm that the delta between both sources is exactly the patch set. > Those ranges are all off-by-one. The first commit should be the one you > want to compare against, not the first one you want to have in the diff. You are right sorry, I meant the commit ranges as inclusive but that's not how "git diff" works. The 10 new commits actually change +222/-17 lines [4]. Emmanuel Bourg [1] https://github.com/AdoptOpenJDK/openjdk-jdk11u/commits/10263b1 [2] https://hg.openjdk.java.net/jdk-updates/jdk11u-dev/shortlog/jdk-11.0.11-ga [3] https://salsa.debian.org/java-team/openjdk-11-jre-dcevm/-/blob/master/debian/patches/hotspot-libpath.diff [4] https://github.com/HotswapProjects/openjdk-jdk11u-dcevm/compare/7e0e338^..28b9ff6
Bug#987658: unblock: openjdk-11-jre-dcevm/11.0.11+9-1
Hi Emmanuel, On 15-05-2021 00:58, Emmanuel Bourg wrote: > I've compared DCEVM 11.0.10 [1] and 11.0.11 [2], Are commits f8bc235 and 10263b1 the head of the OpenJDK archive? Does OpenJDK in Debian carry any patches you want to have too? I start to realize that src:openjdk-11 is probably more than the source of src:openjdk-11-jre-dcevm, as I *assume* that the source of the latter is only a *subset* of the source of the former. What would we need to do to compare those sources? Can you indeed confirm that the delta between both sources is exactly the patch set? > the version 11.0.10 has > 26 commits applied on top of OpenJDK (d0a670c..beb1c75), the version > 11.0.11 has the same 26 commits rebased (ef59517..e27e254) plus 10 new > commits (7e0e338..28b9ff6), they add 215 lines and remove 17. Those ranges are all off-by-one. The first commit should be the one you want to compare against, not the first one you want to have in the diff. Paul OpenPGP_signature Description: OpenPGP digital signature
Bug#987658: unblock: openjdk-11-jre-dcevm/11.0.11+9-1
Hi Paul,
On 14/05/2021 21:46, Paul Gevers wrote:
> I missed so far that apparently DCEVM is a patch set to the same OpenJDK
> we already have in the archive. Is it feasible, i.e. does it make sense
> and would it help us review it, if you generate a diff of patches,
> instead of the source diff that includes all the changes to OpenJDK
> (which we already have in bullseye)?
I've compared DCEVM 11.0.10 [1] and 11.0.11 [2], the version 11.0.10 has
26 commits applied on top of OpenJDK (d0a670c..beb1c75), the version
11.0.11 has the same 26 commits rebased (ef59517..e27e254) plus 10 new
commits (7e0e338..28b9ff6), they add 215 lines and remove 17. I'm
attaching the diff for the extra commits, they can also be reviewed on
GitHub [3].
Emmanuel Bourg
[1]
https://github.com/HotswapProjects/openjdk-jdk11u-dcevm/commits/dcevm-11.0.11+1
[2]
https://github.com/HotswapProjects/openjdk-jdk11u-dcevm/commits/dcevm-11.0.10+1
[3]
https://github.com/HotswapProjects/openjdk-jdk11u-dcevm/compare/7e0e338..28b9ff6
diff --git a/src/hotspot/share/ci/ciKlass.hpp b/src/hotspot/share/ci/ciKlass.hpp
index b15da89732..dc7f05f2c7 100644
--- a/src/hotspot/share/ci/ciKlass.hpp
+++ b/src/hotspot/share/ci/ciKlass.hpp
@@ -131,6 +131,7 @@ public:
const char* external_name() const;
bool is_deoptimization_excl() { return get_Klass()->is_deoptimization_excl(); }
+ Klass* new_version() { return get_Klass()->new_version(); }
};
#endif // SHARE_VM_CI_CIKLASS_HPP
diff --git a/src/hotspot/share/ci/ciObjectFactory.cpp b/src/hotspot/share/ci/ciObjectFactory.cpp
index 66bbe835e7..107c16fa5c 100644
--- a/src/hotspot/share/ci/ciObjectFactory.cpp
+++ b/src/hotspot/share/ci/ciObjectFactory.cpp
@@ -70,7 +70,10 @@ GrowableArray* ciObjectFactory::_shared_ci_metadata = NULL;
ciSymbol* ciObjectFactory::_shared_ci_symbols[vmSymbols::SID_LIMIT];
int ciObjectFactory::_shared_ident_limit = 0;
volatile bool ciObjectFactory::_initialized = false;
+volatile bool ciObjectFactory::_reinitialize_wk_klasses = false;
+// TODO: review...
+Arena* ciObjectFactory::_initial_arena = NULL;
// --
// ciObjectFactory::ciObjectFactory
@@ -112,6 +115,7 @@ void ciObjectFactory::initialize() {
// compiler thread that initializes the initial ciObjectFactory which
// creates the shared ciObjects that all later ciObjectFactories use.
Arena* arena = new (mtCompiler) Arena(mtCompiler);
+ ciObjectFactory::_initial_arena = arena;
ciEnv initial(arena);
ciEnv* env = ciEnv::current();
env->_factory->init_shared_objects();
@@ -120,6 +124,36 @@ void ciObjectFactory::initialize() {
}
+// (DCEVM) wk classes could be modified
+void ciObjectFactory::reinitialize_wk_classes() {
+ ASSERT_IN_VM;
+ JavaThread* thread = JavaThread::current();
+ HandleMark handle_mark(thread);
+
+ // This Arena is long lived and exists in the resource mark of the
+ // compiler thread that initializes the initial ciObjectFactory which
+ // creates the shared ciObjects that all later ciObjectFactories use.
+ // Arena* arena = new (mtCompiler) Arena(mtCompiler);
+ ciEnv initial(ciObjectFactory::_initial_arena);
+ ciEnv* env = ciEnv::current();
+ env->_factory->do_reinitialize_wk_classes();
+ _reinitialize_wk_klasses = false;
+}
+
+// (DCEVM) wk classes could be modified
+void ciObjectFactory::do_reinitialize_wk_classes() {
+#define WK_KLASS_DEFN(name, ignore_s, opt) \
+ if (ciEnv::_##name != NULL && ciEnv::_##name->new_version() != NULL) { \
+int old_ident = ciEnv::_##name->ident(); \
+ciEnv::_##name = get_metadata(SystemDictionary::name())->as_instance_klass(); \
+ciEnv::_##name->compute_nonstatic_fields(); \
+ciEnv::_##name->set_ident(old_ident); \
+ }
+
+ WK_KLASSES_DO(WK_KLASS_DEFN)
+#undef WK_KLASS_DEFN
+}
+
void ciObjectFactory::init_shared_objects() {
_next_ident = 1; // start numbering CI objects at 1
diff --git a/src/hotspot/share/ci/ciObjectFactory.hpp b/src/hotspot/share/ci/ciObjectFactory.hpp
index 3e9d48c4cd..79059f6e2e 100644
--- a/src/hotspot/share/ci/ciObjectFactory.hpp
+++ b/src/hotspot/share/ci/ciObjectFactory.hpp
@@ -41,9 +41,11 @@ class ciObjectFactory : public ResourceObj {
private:
static volatile bool _initialized;
+ static volatile bool _reinitialize_wk_klasses;
static GrowableArray* _shared_ci_metadata;
static ciSymbol* _shared_ci_symbols[];
static int _shared_ident_limit;
+ static Arena*_initial_arena;
Arena*_arena;
GrowableArray*_ci_metadata;
@@ -89,10 +91,14 @@ private:
ciInstance* get_unloaded_instance(ciInstanceKlass* klass);
static int compare_cimetadata(ciMetadata** a, ciMetadata** b);
+ void do_reinitialize_wk_classes();
public:
static bool is_initialized() { return _initialized; }
+ static bool is_reinitialize_wk_klasses() { return _reinitialize_wk_klasses; }
Bug#987658: unblock: openjdk-11-jre-dcevm/11.0.11+9-1
Hi Emmanuel, On 01-05-2021 14:43, Emmanuel Bourg wrote: > I agree the diff is not reviewable, but it can be seen as an update of > the DCEVM code to the same state as the OpenJDK code that was already > accepted in testing. The extra DCEVM patches were simply rebased with no > changes on top of OpenJDK 11.0.11 [1]. I missed so far that apparently DCEVM is a patch set to the same OpenJDK we already have in the archive. Is it feasible, i.e. does it make sense and would it help us review it, if you generate a diff of patches, instead of the source diff that includes all the changes to OpenJDK (which we already have in bullseye)? Paul OpenPGP_signature Description: OpenPGP digital signature
Bug#987658: unblock: openjdk-11-jre-dcevm/11.0.11+9-1
Le 03/05/2021 à 10:15, Emmanuel Bourg a écrit : >> If your package is so tightly tied to the version of openjdk I would >> expect mechanisms to be in place to warn you of the upload and to prevent >> migration of openjdk-11 in case of breakage. E.g. autopkgtest is very >> well suited to prevent the migration because it's integrated with our >> migration software. Having an autopkgtest has the additional benefit >> that at this stage of the release, you're package isn't blocked if it >> passes on all architecture and you wouldn't need to bother us with an >> unblock request. > > I agree that a better QA mechanism would be good. I'd be a bit reluctant > to implement autopkgtest though, my understanding is that openjdk-11 > would be blocked from migrating if it breaks openjdk-11-jre-dcevm, but > if DCEVM upstream lags to rebase its patches that would be unfair to > delay the OpenJDK update, because OpenJDK has a much larger user base > than DCEVM and the benefits of an OpenJDK update outweigh a temporary > breakage of DCEVM. I've noticed that openjdk-11 already has two autopkgtest failures reported and they seem to be ignored. So I went ahead and uploaded openjdk-11-jre-dcevm/11.0.11+9-2 with a simple autopkgtest test suite that should catch similar failures in the future. Emmanuel Bourg
Bug#987658: unblock: openjdk-11-jre-dcevm/11.0.11+9-1
Hi Paul, Le 02/05/2021 à 21:38, Paul Gevers a écrit : > But OpenJDK 11.0.11 was uploaded two months ago, when we were still in > the soft freeze. Why didn't you fix openjdk-11-jre-dcevm then? OpenJDK 11.0.11+3 was uploaded in February, but this wasn't a GA release, OpenJDK 11.0.11 was only officially released two weeks ago [1]. DCEVM upstream tends to only track the official releases (I don't think non GA releases of OpenJDK should ever be in testing but that's another debate). So I was in a bind until April 27th when DCEVM was updated, one week after the 11.0.11 GA release. And I filed this unblock request the very same day. > If your package is so tightly tied to the version of openjdk I would > expect mechanisms to be in place to warn you of the upload and to prevent > migration of openjdk-11 in case of breakage. E.g. autopkgtest is very > well suited to prevent the migration because it's integrated with our > migration software. Having an autopkgtest has the additional benefit > that at this stage of the release, you're package isn't blocked if it > passes on all architecture and you wouldn't need to bother us with an > unblock request. I agree that a better QA mechanism would be good. I'd be a bit reluctant to implement autopkgtest though, my understanding is that openjdk-11 would be blocked from migrating if it breaks openjdk-11-jre-dcevm, but if DCEVM upstream lags to rebase its patches that would be unfair to delay the OpenJDK update, because OpenJDK has a much larger user base than DCEVM and the benefits of an OpenJDK update outweigh a temporary breakage of DCEVM. > How do you normally get triggered to update you package? I scan my DDPO [2] every week and look for upstream updates. > Two months ago, in a different stage of the release. I understand this unblock request bends the rules and I'm really sorry for that. But that's really the best I can do to serve our users. The risk is limited since this is a minor update of a leaf package that trades a totally broken version for a good one, so I thought it was worth considering for an exception. Emmanuel Bourg [1] https://wiki.openjdk.java.net/display/JDKUpdates/JDK11u [2] https://qa.debian.org/developer.php?login=pkg-java-maintain...@lists.alioth.debian.org
Bug#987658: unblock: openjdk-11-jre-dcevm/11.0.11+9-1
Hi Emmanuel On 01-05-2021 14:43, Emmanuel Bourg wrote: > Every time OpenJDK is > updated in Debian, the corresponding DCEVM package has to be updated as > well, otherwise it's likely to fail or crash. That's exactly what > happens currently in testing, we have OpenJDK 11.0.11 with DCEVM > 11.0.10, and DCEVM just crashes (a simple invocation of "java -dcevm > -version" throws an error). But OpenJDK 11.0.11 was uploaded two months ago, when we were still in the soft freeze. Why didn't you fix openjdk-11-jre-dcevm then? If your package is so tightly tied to the version of openjdk I would expect mechanisms to be in place to warn you of the upload and to prevent migration of openjdk-11 in case of breakage. E.g. autopkgtest is very well suited to prevent the migration because it's integrated with our migration software. Having an autopkgtest has the additional benefit that at this stage of the release, you're package isn't blocked if it passes on all architecture and you wouldn't need to bother us with an unblock request. How do you normally get triggered to update you package? > I agree the diff is not reviewable, but it can be seen as an update of > the DCEVM code to the same state as the OpenJDK code that was already > accepted in testing. Two months ago, in a different stage of the release. Paul OpenPGP_signature Description: OpenPGP digital signature
Bug#987658: unblock: openjdk-11-jre-dcevm/11.0.11+9-1
Le 30/04/2021 à 21:39, Paul Gevers a écrit : >> Please unblock package openjdk-11-jre-dcevm > > 333 files changed, 8389 insertions(+), 2196 deletions(-) > That's not reviewable. > >> openjdk-11-jre-dcevm/11.0.10+1-1 in testing is currently unusable, it >> throws an error because the version isn't aligned with the openjdk-11 >> package (#984725). > > Can't that bug be fixed by cherry-picking? A new upstream is not > acceptable like this at this stage of the release. Please read our FAQ > [1] and act accordingly. Hi Paul, Thank you for looking into this. DCEVM is a patched HotSpot JVM with extra features useful to developers (it supports more hot reloading changes in debug mode than the standard JVM). Every time OpenJDK is updated in Debian, the corresponding DCEVM package has to be updated as well, otherwise it's likely to fail or crash. That's exactly what happens currently in testing, we have OpenJDK 11.0.11 with DCEVM 11.0.10, and DCEVM just crashes (a simple invocation of "java -dcevm -version" throws an error). I agree the diff is not reviewable, but it can be seen as an update of the DCEVM code to the same state as the OpenJDK code that was already accepted in testing. The extra DCEVM patches were simply rebased with no changes on top of OpenJDK 11.0.11 [1]. I'm afraid the JVM is too complex for a mere mortal like me to cherry-pick the right changes and be confident the result isn't broken in some ways. Without this update the dcevm package is broken and will have to be fixed by a stable or security update (or removed from testing, but that would be sad for the developers using it). Emmanuel Bourg [1] https://github.com/HotswapProjects/openjdk-jdk11u-dcevm
Bug#987658: unblock: openjdk-11-jre-dcevm/11.0.11+9-1
Control: tags -1 moreinfo Hi Emmanuel, On 27-04-2021 13:47, Emmanuel Bourg wrote: > Please unblock package openjdk-11-jre-dcevm 333 files changed, 8389 insertions(+), 2196 deletions(-) That's not reviewable. > openjdk-11-jre-dcevm/11.0.10+1-1 in testing is currently unusable, it > throws an error because the version isn't aligned with the openjdk-11 > package (#984725). Can't that bug be fixed by cherry-picking? A new upstream is not acceptable like this at this stage of the release. Please read our FAQ [1] and act accordingly. Paul [1] https://release.debian.org/bullseye/FAQ.html OpenPGP_signature Description: OpenPGP digital signature
Bug#987658: unblock: openjdk-11-jre-dcevm/11.0.11+9-1
Package: release.debian.org Severity: normal User: release.debian@packages.debian.org Usertags: unblock Please unblock package openjdk-11-jre-dcevm openjdk-11-jre-dcevm/11.0.10+1-1 in testing is currently unusable, it throws an error because the version isn't aligned with the openjdk-11 package (#984725). This update is compatible with both versions of openjdk-11 in testing (11.0.11+4-1) and unstable (11.0.11+9-1). Thank you, Emmanuel Bourg unblock openjdk-11-jre-dcevm/11.0.11+9-1
