Re: RFR: 8309044: Replace NULL with nullptr, final sweep of hotspot code [v2]

2023-06-01 Thread Tobias Hartmann
On Tue, 30 May 2023 19:15:38 GMT, Johan Sjölen wrote: >> A final sweep of Hotspot to remove all re-added NULLs. With only 110 changes >> I'd appreciate if this was considered trivial. > > Johan Sjölen has updated the pull request incrementally with two additional > commits since the last

Re: RFR: 8309329: com/sun/jdi/DeferredStepTest.java fails with virtual threads due to not waiting for threads to exit [v2]

2023-06-01 Thread Chris Plummer
> Virtual threads are always daemon threads, so tests that previously did not > explicitly wait for test threads to exit sometimes fail with virtual threads > due to the test exiting before the test threads have exited. A join() for > each test thread is needed to fix this issue. > >

Re: RFR: 8304438: jcmd JVMTI.agent_load should obey EnableDynamicAgentLoading [v6]

2023-06-01 Thread Chris Plummer
On Fri, 2 Jun 2023 03:44:48 GMT, Serguei Spitsyn wrote: >> The VM option EnableDynamicAgentLoading was added in JDK 9, default true, to >> allow deployment to choose whether to allow agents to be loaded/started in >> the VM. The VM option does the right thing for tools using the Attach API >>

Re: RFR: 8304438: jcmd JVMTI.agent_load should obey EnableDynamicAgentLoading [v6]

2023-06-01 Thread Serguei Spitsyn
> The VM option EnableDynamicAgentLoading was added in JDK 9, default true, to > allow deployment to choose whether to allow agents to be loaded/started in > the VM. The VM option does the right thing for tools using the Attach API but > jcmd JVMTI.agent_load was missed. This should be fixed to

Re: RFR: 8309329: com/sun/jdi/DeferredStepTest.java fails with virtual threads due to not waiting for threads to exit

2023-06-01 Thread Serguei Spitsyn
On Thu, 1 Jun 2023 23:03:47 GMT, Chris Plummer wrote: > Virtual threads are always daemon threads, so tests that previously did not > explicitly wait for test threads to exit sometimes fail with virtual threads > due to the test exiting before the test threads have exited. A join() for > each

Re: RFR: 8304438: jcmd JVMTI.agent_load should obey EnableDynamicAgentLoading [v5]

2023-06-01 Thread Serguei Spitsyn
> The VM option EnableDynamicAgentLoading was added in JDK 9, default true, to > allow deployment to choose whether to allow agents to be loaded/started in > the VM. The VM option does the right thing for tools using the Attach API but > jcmd JVMTI.agent_load was missed. This should be fixed to

Re: RFR: 8304438: jcmd JVMTI.agent_load should obey EnableDynamicAgentLoading [v4]

2023-06-01 Thread Serguei Spitsyn
On Thu, 1 Jun 2023 23:14:23 GMT, Chris Plummer wrote: >> Serguei Spitsyn has updated the pull request incrementally with one >> additional commit since the last revision: >> >> review: use output.shouldContain() > > test/jdk/sun/tools/jcmd/TestJcmdNoAgentLoad.java line 57: > >> 55:

Re: RFR: JDK-8307314: Implementation: Generational Shenandoah (Experimental) [v4]

2023-06-01 Thread Kelvin Nilsen
On Thu, 1 Jun 2023 14:27:12 GMT, Thomas Stuefe wrote: >> Kelvin Nilsen has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Force PLAB sizes to align on card-table size > > test/hotspot/jtreg/gc/shenandoah/oom/TestClassLoaderLeak.java line

Re: RFR: JDK-8307314: Implementation: Generational Shenandoah (Experimental) [v4]

2023-06-01 Thread Kelvin Nilsen
On Thu, 1 Jun 2023 18:39:05 GMT, William Kemper wrote: >> src/hotspot/share/gc/shared/gcConfiguration.cpp line 88: >> >>> 86: } >>> 87: #endif >>> 88: return NA; >> >> You moved the order between Shenandoah and ZGC in `young_collector()`, so >> you should probably do the same here. >

Re: RFR: JDK-8307314: Implementation: Generational Shenandoah (Experimental) [v4]

2023-06-01 Thread Kelvin Nilsen
> OpenJDK Colleagues: > > Please review this proposed integration of Generational mode for Shenandoah > GC under https://bugs.openjdk.org/browse/JDK-8307314. > > Generational mode of Shenandoah is enabled by adding > `-XX:+UnlockExperimentalVMOptions -XX:ShenandoahGCMode=generational` to a >

Re: RFR: JDK-8307314: Implementation: Generational Shenandoah (Experimental) [v3]

2023-06-01 Thread Kelvin Nilsen
> OpenJDK Colleagues: > > Please review this proposed integration of Generational mode for Shenandoah > GC under https://bugs.openjdk.org/browse/JDK-8307314. > > Generational mode of Shenandoah is enabled by adding > `-XX:+UnlockExperimentalVMOptions -XX:ShenandoahGCMode=generational` to a >

Re: RFR: 8309329: com/sun/jdi/DeferredStepTest.java fails with virtual threads due to not waiting for threads to exit

2023-06-01 Thread Alex Menkov
On Thu, 1 Jun 2023 23:03:47 GMT, Chris Plummer wrote: > Virtual threads are always daemon threads, so tests that previously did not > explicitly wait for test threads to exit sometimes fail with virtual threads > due to the test exiting before the test threads have exited. A join() for > each

Re: RFR: 8308978: regression with a deadlock involving FollowReferences [v3]

2023-06-01 Thread Alex Menkov
On Wed, 31 May 2023 21:33:13 GMT, Alex Menkov wrote: >> The change fixes regression from JDK-8299414. >> There is a deadlock between JvmtiVTMSTransitionDisabler and EscapeBarrier >> when virtual threads are in mount/unmount transition: >> EscapeBarrier requests deoptimization which requires

Re: RFR: 8304438: jcmd JVMTI.agent_load should obey EnableDynamicAgentLoading [v4]

2023-06-01 Thread Chris Plummer
On Thu, 1 Jun 2023 22:51:23 GMT, Serguei Spitsyn wrote: >> The VM option EnableDynamicAgentLoading was added in JDK 9, default true, to >> allow deployment to choose whether to allow agents to be loaded/started in >> the VM. The VM option does the right thing for tools using the Attach API >>

RFR: 8309329: com/sun/jdi/DeferredStepTest.java fails with virtual threads due to not waiting for threads to exit

2023-06-01 Thread Chris Plummer
Virtual threads are always daemon threads, so tests that previously did not explicitly wait for test threads to exit sometimes fail with virtual threads due to the test exiting before the test threads have exited. A join() for each test thread is needed to fix this issue.

Re: RFR: 8304438: jcmd JVMTI.agent_load should obey EnableDynamicAgentLoading [v4]

2023-06-01 Thread Serguei Spitsyn
> The VM option EnableDynamicAgentLoading was added in JDK 9, default true, to > allow deployment to choose whether to allow agents to be loaded/started in > the VM. The VM option does the right thing for tools using the Attach API but > jcmd JVMTI.agent_load was missed. This should be fixed to

Re: RFR: 8304438: jcmd JVMTI.agent_load should obey EnableDynamicAgentLoading [v2]

2023-06-01 Thread Serguei Spitsyn
On Wed, 31 May 2023 22:08:43 GMT, Chris Plummer wrote: >> Serguei Spitsyn has updated the pull request incrementally with one >> additional commit since the last revision: >> >> minor renaming in new test TestJcmdNoAgentLoad.java > > test/jdk/sun/tools/jcmd/TestJcmdNoAgentLoad.java line 68:

Re: RFR: 8308090: Add container tests for on-the-fly resource quota updates [v2]

2023-06-01 Thread Mikhailo Seledtsov
On Tue, 23 May 2023 09:04:11 GMT, Severin Gehwolf wrote: >> Please review these test changes which implement automatic testing of >> container resource updates without JVM restart. Note that this merely tests >> container detection code handling this case. It doesn't do anything special >>

Re: RFR: 8307478: Implementation of Prepare to Restrict The Dynamic Loading of Agents [v9]

2023-06-01 Thread Chris Plummer
On Thu, 1 Jun 2023 16:56:17 GMT, Alan Bateman wrote: >> This is the implementation for JEP 451. There are two parts to this: >> >> 1. A multi-line warning is printed when a JVM TI or Java agent is loaded >> into a running VM. For JVM TI, the message is printed to stderr from >>

Re: RFR: JDK-8307314: Implementation: Generational Shenandoah (Experimental) [v2]

2023-06-01 Thread William Kemper
On Thu, 1 Jun 2023 10:12:02 GMT, Stefan Karlsson wrote: >> Kelvin Nilsen has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Make the order of young/old collector checks consistent (#1) > > src/hotspot/share/gc/shared/gcConfiguration.cpp

Re: RFR: JDK-8307314: Implementation: Generational Shenandoah (Experimental) [v2]

2023-06-01 Thread Kelvin Nilsen
> OpenJDK Colleagues: > > Please review this proposed integration of Generational mode for Shenandoah > GC under https://bugs.openjdk.org/browse/JDK-8307314. > > Generational mode of Shenandoah is enabled by adding > `-XX:+UnlockExperimentalVMOptions -XX:ShenandoahGCMode=generational` to a >

Re: RFR: JDK-8307314: Implementation: Generational Shenandoah (Experimental)

2023-06-01 Thread Y . Srinivas Ramakrishna
On Thu, 1 Jun 2023 12:15:37 GMT, Martin Doerr wrote: > Issues already reported to GenShen engineers: > > gc/shenandoah/TestElasticTLAB.java#generational #  Internal Error > (src\hotspot\share\gc\shenandoah\shenandoahFreeSet.cpp:695), pid=23288, > tid=23784 #  assert(size %

Re: RFR: 8307478: Implementation of Prepare to Restrict The Dynamic Loading of Agents [v9]

2023-06-01 Thread Alan Bateman
> This is the implementation for JEP 451. There are two parts to this: > > 1. A multi-line warning is printed when a JVM TI or Java agent is loaded into > a running VM. For JVM TI, the message is printed to stderr from > JvmtiAgent::load. For Java agents, it is printed to System.err (as that

Re: RFR: 8307478: Implementation of Prepare to Restrict The Dynamic Loading of Agents [v8]

2023-06-01 Thread Chris Plummer
On Thu, 1 Jun 2023 12:37:20 GMT, Alan Bateman wrote: >> This is the implementation for JEP 451. There are two parts to this: >> >> 1. A multi-line warning is printed when a JVM TI or Java agent is loaded >> into a running VM. For JVM TI, the message is printed to stderr from >>

Re: RFR: 8304438: jcmd JVMTI.agent_load should obey EnableDynamicAgentLoading [v3]

2023-06-01 Thread Chris Plummer
On Wed, 31 May 2023 23:39:19 GMT, Serguei Spitsyn wrote: >> The VM option EnableDynamicAgentLoading was added in JDK 9, default true, to >> allow deployment to choose whether to allow agents to be loaded/started in >> the VM. The VM option does the right thing for tools using the Attach API

Integrated: 8309146: extend JDI StackFrame.setValue() and JDWP StackFrame.setValues minimal support for virtual threads

2023-06-01 Thread Chris Plummer
On Tue, 30 May 2023 23:44:28 GMT, Chris Plummer wrote: > The JDWP spec for StackFrame.SetValue currently states: > > "If the thread is a virtual thread then this command can be used to > set " > "the value of local variables in the top-most frame when the thread > is " >

Integrated: 8308232: nsk/jdb tests don't pass -verbose flag to the debuggee

2023-06-01 Thread Chris Plummer
On Wed, 31 May 2023 00:27:16 GMT, Chris Plummer wrote: > The nsk/jdb tests are not passing the test args on to the debuggee. I found a > way to pass all of them (see the CR for details), but Windows had trouble > with the parsing. It turns out the only args that the debuggee really cares >

Re: RFR: 8308232: nsk/jdb tests don't pass -verbose flag to the debuggee

2023-06-01 Thread Chris Plummer
On Wed, 31 May 2023 00:27:16 GMT, Chris Plummer wrote: > The nsk/jdb tests are not passing the test args on to the debuggee. I found a > way to pass all of them (see the CR for details), but Windows had trouble > with the parsing. It turns out the only args that the debuggee really cares >

Re: RFR: 8309146: extend JDI StackFrame.setValue() and JDWP StackFrame.setValues minimal support for virtual threads

2023-06-01 Thread Alan Bateman
On Tue, 30 May 2023 23:44:28 GMT, Chris Plummer wrote: > The JDWP spec for StackFrame.SetValue currently states: > > "If the thread is a virtual thread then this command can be used to > set " > "the value of local variables in the top-most frame when the thread > is " >

Re: RFR: 8304438: jcmd JVMTI.agent_load should obey EnableDynamicAgentLoading [v3]

2023-06-01 Thread Chris Plummer
On Wed, 31 May 2023 23:39:19 GMT, Serguei Spitsyn wrote: >> The VM option EnableDynamicAgentLoading was added in JDK 9, default true, to >> allow deployment to choose whether to allow agents to be loaded/started in >> the VM. The VM option does the right thing for tools using the Attach API

Re: RFR: 8304438: jcmd JVMTI.agent_load should obey EnableDynamicAgentLoading [v2]

2023-06-01 Thread Chris Plummer
On Wed, 31 May 2023 10:30:22 GMT, Serguei Spitsyn wrote: >> The VM option EnableDynamicAgentLoading was added in JDK 9, default true, to >> allow deployment to choose whether to allow agents to be loaded/started in >> the VM. The VM option does the right thing for tools using the Attach API

Re: RFR: 8307478: Implementation of Prepare to Restrict The Dynamic Loading of Agents [v7]

2023-06-01 Thread Chris Plummer
On Thu, 1 Jun 2023 06:46:41 GMT, Serguei Spitsyn wrote: >>> The use of `!JvmtiAgentList::is_loaded(library)` here is a bit confusing. >>> Isn't the library always already loaded by the time we get here (the assert >>> below seems to imply that)? >> >> JvmtiAgentList::load_agent creates the

Re: RFR: 8308286 Fix clang warnings in linux code [v3]

2023-06-01 Thread Weijun Wang
On Thu, 1 Jun 2023 15:02:16 GMT, Artem Semenov wrote: >> src/java.security.jgss/share/native/libj2gss/gssapi.h line 47: >> >>> 45: >>> 46: // Condition was copied from >>> 47: // >>> Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk/usr/include/gssapi/gssapi.h

Re: RFR: 8308286 Fix clang warnings in linux code [v3]

2023-06-01 Thread Artem Semenov
On Wed, 31 May 2023 13:52:39 GMT, Weijun Wang wrote: >> Artem Semenov has updated the pull request incrementally with one additional >> commit since the last revision: >> >> update > > src/java.security.jgss/share/native/libj2gss/gssapi.h line 47: > >> 45: >> 46: // Condition was copied

Re: RFR: 8308286 Fix clang warnings in linux code [v4]

2023-06-01 Thread Artem Semenov
> When using the clang compiler to build OpenJDk on Linux, we encounter various > "warnings as errors". > They can be fixed with small changes. Artem Semenov has updated the pull request incrementally with one additional commit since the last revision: update - Changes: -

Re: RFR: 8307478: Implementation of Prepare to Restrict The Dynamic Loading of Agents [v8]

2023-06-01 Thread Serguei Spitsyn
On Thu, 1 Jun 2023 12:37:20 GMT, Alan Bateman wrote: >> This is the implementation for JEP 451. There are two parts to this: >> >> 1. A multi-line warning is printed when a JVM TI or Java agent is loaded >> into a running VM. For JVM TI, the message is printed to stderr from >>

Re: RFR: JDK-8307314: Implementation: Generational Shenandoah (Experimental)

2023-06-01 Thread Thomas Stuefe
On Fri, 26 May 2023 20:46:29 GMT, Kelvin Nilsen wrote: > OpenJDK Colleagues: > > Please review this proposed integration of Generational mode for Shenandoah > GC under https://bugs.openjdk.org/browse/JDK-8307314. > > Generational mode of Shenandoah is enabled by adding >

Re: RFR: JDK-8308288: Fix xlc17 clang warnings in shared code [v2]

2023-06-01 Thread JoKern65
On Fri, 26 May 2023 08:31:46 GMT, JoKern65 wrote: >> When using the new xlc17 compiler (based on a recent clang) to build OpenJDk >> on AIX , we run into various "warnings as errors". >> Some of those are in shared codebase and could be addressed by small >> adjustments. >> A lot of those

Withdrawn: JDK-8308288: Fix xlc17 clang warnings in shared code

2023-06-01 Thread JoKern65
On Thu, 25 May 2023 09:14:14 GMT, JoKern65 wrote: > When using the new xlc17 compiler (based on a recent clang) to build OpenJDk > on AIX , we run into various "warnings as errors". > Some of those are in shared codebase and could be addressed by small > adjustments. > A lot of those changes

Re: RFR: JDK-8307314: Implementation: Generational Shenandoah (Experimental)

2023-06-01 Thread Thomas Stuefe
On Fri, 26 May 2023 20:46:29 GMT, Kelvin Nilsen wrote: > OpenJDK Colleagues: > > Please review this proposed integration of Generational mode for Shenandoah > GC under https://bugs.openjdk.org/browse/JDK-8307314. > > Generational mode of Shenandoah is enabled by adding >

Re: RFR: 8307478: Implementation of Prepare to Restrict The Dynamic Loading of Agents [v6]

2023-06-01 Thread Alan Bateman
On Thu, 1 Jun 2023 05:55:43 GMT, Alan Bateman wrote: >> src/hotspot/share/prims/jvmtiAgentList.cpp line 231: >> >>> 229:if (agent->is_static_lib() && agent->is_loaded()) { >>> 230: return true; >>> 231:} >> >> This doesn't make sense to me. If you pass in `null` for

Re: RFR: 8307478: Implementation of Prepare to Restrict The Dynamic Loading of Agents [v8]

2023-06-01 Thread Alan Bateman
> This is the implementation for JEP 451. There are two parts to this: > > 1. A multi-line warning is printed when a JVM TI or Java agent is loaded into > a running VM. For JVM TI, the message is printed to stderr from > JvmtiAgent::load. For Java agents, it is printed to System.err (as that

Re: RFR: JDK-8307314: Implementation: Generational Shenandoah (Experimental)

2023-06-01 Thread Martin Doerr
On Fri, 26 May 2023 20:46:29 GMT, Kelvin Nilsen wrote: > OpenJDK Colleagues: > > Please review this proposed integration of Generational mode for Shenandoah > GC under https://bugs.openjdk.org/browse/JDK-8307314. > > Generational mode of Shenandoah is enabled by adding >

Re: RFR: JDK-8307314: Implementation: Generational Shenandoah (Experimental)

2023-06-01 Thread Stefan Karlsson
On Fri, 26 May 2023 20:46:29 GMT, Kelvin Nilsen wrote: > OpenJDK Colleagues: > > Please review this proposed integration of Generational mode for Shenandoah > GC under https://bugs.openjdk.org/browse/JDK-8307314. > > Generational mode of Shenandoah is enabled by adding >

Re: RFR: JDK-8307314: Implementation: Generational Shenandoah (Experimental)

2023-06-01 Thread Andrew Haley
On Fri, 26 May 2023 20:46:29 GMT, Kelvin Nilsen wrote: > OpenJDK Colleagues: > > Please review this proposed integration of Generational mode for Shenandoah > GC under https://bugs.openjdk.org/browse/JDK-8307314. > > Generational mode of Shenandoah is enabled by adding >

Re: RFR: 8309044: Replace NULL with nullptr, final sweep of hotspot code [v2]

2023-06-01 Thread Doug Simon
On Tue, 30 May 2023 19:15:38 GMT, Johan Sjölen wrote: >> A final sweep of Hotspot to remove all re-added NULLs. With only 110 changes >> I'd appreciate if this was considered trivial. > > Johan Sjölen has updated the pull request incrementally with two additional > commits since the last

Re: RFR: 8309044: Replace NULL with nullptr, final sweep of hotspot code [v2]

2023-06-01 Thread Johan Sjölen
On Thu, 1 Jun 2023 05:23:25 GMT, Tobias Hartmann wrote: > What's the plan now to prevent re-introducing `NULL`? Hi Tobias. The only plan in place is social, the reviewers have to look out for it. I am however researching how to do this through machine. I'm currently researching ways of

Re: RFR: 8309044: Replace NULL with nullptr, final sweep of hotspot code [v2]

2023-06-01 Thread Andrew Haley
On Tue, 30 May 2023 00:57:40 GMT, David Holmes wrote: > Can we now poison NULL so it can't get reintroduced? Or would that > potentially break standard headers? I'm sure it would. Maybe some changes to Skara? - PR Comment:

RFR: 8309271: A way to align already compiled methods with compiler directives

2023-06-01 Thread Dmitry Chuyko
Compiler Control (https://openjdk.org/jeps/165) provides method-context dependent control of the JVM compilers (C1 and C2). The active directive stack is built from the directive files passed with the `-XX:CompilerDirectivesFile` diagnostic command-line option and the Compiler.add_directives

Re: Clarifying jmethodID Usage and Potential JVM Crashes

2023-06-01 Thread David Holmes
On 1/06/2023 5:16 pm, Jaroslav Bachorík wrote: Hi David, On Thu, Jun 1, 2023 at 3:56 AM David Holmes > wrote: Hi Jaroslav, On 31/05/2023 9:12 pm, Jaroslav Bachorík wrote: > Dear Team, > > I've been investigating the unusual JVM crashes

Re: RFR: 8308090: Add container tests for on-the-fly resource quota updates [v2]

2023-06-01 Thread Severin Gehwolf
On Thu, 1 Jun 2023 02:16:12 GMT, David Holmes wrote: >> Anyone willing to review this? > > @jerboaa I can't really review the tests themselves but will run through our > CI to see if they cause any problems. If not then they should be okay to add. Thanks @dholmes-ora for running them through

Re: Clarifying jmethodID Usage and Potential JVM Crashes

2023-06-01 Thread Thomas Stüfe
On Thu, Jun 1, 2023 at 9:17 AM Jaroslav Bachorík < jaroslav.bacho...@datadoghq.com> wrote: > Hi David, > > On Thu, Jun 1, 2023 at 3:56 AM David Holmes > wrote: > >> Hi Jaroslav, >> >> On 31/05/2023 9:12 pm, Jaroslav Bachorík wrote: >> > Dear Team, >> > >> > I've been investigating the unusual

Re: RFR: 8307478: Implementation of Prepare to Restrict The Dynamic Loading of Agents [v7]

2023-06-01 Thread Serguei Spitsyn
On Wed, 31 May 2023 15:01:37 GMT, Alan Bateman wrote: >> This is the implementation for JEP 451. There are two parts to this: >> >> 1. A multi-line warning is printed when a JVM TI or Java agent is loaded >> into a running VM. For JVM TI, the message is printed to stderr from >>

Re: Clarifying jmethodID Usage and Potential JVM Crashes

2023-06-01 Thread Jaroslav Bachorík
Hi David, On Thu, Jun 1, 2023 at 3:56 AM David Holmes wrote: > Hi Jaroslav, > > On 31/05/2023 9:12 pm, Jaroslav Bachorík wrote: > > Dear Team, > > > > I've been investigating the unusual JVM crashes occurring in JVMTI calls > > on a J9 JVM. During my investigation, I scrutinized the `jmethodID`

Re: RFR: 8307478: Implementation of Prepare to Restrict The Dynamic Loading of Agents [v7]

2023-06-01 Thread Serguei Spitsyn
On Thu, 1 Jun 2023 05:51:51 GMT, Alan Bateman wrote: >> It looks like you are right. >> There is also and assert at line 519: >> `519 assert(agent->is_loaded(), "invariant");` >> >> So, the agent has to be loaded if we got to the line 512. >> Also, there is a statement at line 507 (before

Re: RFR: 8308090: Add container tests for on-the-fly resource quota updates [v2]

2023-06-01 Thread David Holmes
On Tue, 23 May 2023 09:04:11 GMT, Severin Gehwolf wrote: >> Please review these test changes which implement automatic testing of >> container resource updates without JVM restart. Note that this merely tests >> container detection code handling this case. It doesn't do anything special >>

Re: RFR: 8307478: Implementation of Prepare to Restrict The Dynamic Loading of Agents [v4]

2023-06-01 Thread Alan Bateman
On Wed, 31 May 2023 22:13:08 GMT, Chris Plummer wrote: >> We can't currently test `jcmd JVMTI.agent_load` with >> `-XX:-EnableDynamicAgentLoading` due to JDK-8304438. But this test is for >> warnings, not that dynamic loading is disallowed. > > @AlanBateman @sspitsyn Can this be revisited now