[GitHub] [cxf] rmannibucau commented on pull request #721: [cxf-8340] add precompiled class for Graalvm native support

2020-12-17 Thread GitBox
rmannibucau commented on pull request #721: URL: https://github.com/apache/cxf/pull/721#issuecomment-747307674 shouldnt the doc mention jaxrs too? This is an automated message from the Apache Git Service. To respond to the

[GitHub] [cxf] rmannibucau commented on pull request #721: [cxf-8340] add precompiled class for Graalvm native support

2020-12-07 Thread GitBox
rmannibucau commented on pull request #721: URL: https://github.com/apache/cxf/pull/721#issuecomment-739840070 Doc looks ok (https://github.com/apache/cxf/pull/721/commits/72930d287d6e94e6ed742331bbf4f0d2ce645129#diff-d4ccfe1d5ea99f1cf8fc897e745c724ec4a2e2bdbff690a2f088989871f20e4bR21

[GitHub] [cxf] rmannibucau commented on pull request #721: [cxf-8340] add precompiled class for Graalvm native support

2020-12-06 Thread GitBox
rmannibucau commented on pull request #721: URL: https://github.com/apache/cxf/pull/721#issuecomment-739732809 Except ClassGeneratorClassLoader static variables which still look fishy and unexpected - they hide another bug to be concrete like during redeployment of a server with some

[GitHub] [cxf] rmannibucau commented on pull request #721: [cxf-8340] add precompiled class for Graalvm native support

2020-11-28 Thread GitBox
rmannibucau commented on pull request #721: URL: https://github.com/apache/cxf/pull/721#issuecomment-735285989 While there is a clear path for build time generation without quarkus and (worse case) with a small custom main im fine with a smaller first step (we should probably document it

[GitHub] [cxf] rmannibucau commented on pull request #721: [cxf-8340] add precompiled class for Graalvm native support

2020-11-28 Thread GitBox
rmannibucau commented on pull request #721: URL: https://github.com/apache/cxf/pull/721#issuecomment-735072209 @dufoli I'd really drop it since it has a lot of pitfall. You can have one implementation which would use it as non static attribute and keep the default one having a single map

[GitHub] [cxf] rmannibucau commented on pull request #721: [cxf-8340] add precompiled class for Graalvm native support

2020-11-24 Thread GitBox
rmannibucau commented on pull request #721: URL: https://github.com/apache/cxf/pull/721#issuecomment-733204047 @dufoli LC_ALL=en_US.UTF-8 ? This is an automated message from the Apache Git Service. To respond to the message,

[GitHub] [cxf] rmannibucau commented on pull request #721: [cxf-8340] add precompiled class for Graalvm native support

2020-11-23 Thread GitBox
rmannibucau commented on pull request #721: URL: https://github.com/apache/cxf/pull/721#issuecomment-732147376 @dufoli it is not needed, think a final "ultra clean" patch can be to force visitMaxs(0, 0) call on visitEnd() one and drop all ref to visitMaxs but if it works it is good for

[GitHub] [cxf] rmannibucau commented on pull request #721: [cxf-8340] add precompiled class for Graalvm native support

2020-11-16 Thread GitBox
rmannibucau commented on pull request #721: URL: https://github.com/apache/cxf/pull/721#issuecomment-728084199 @reta since "1." and "2." are not in this PR - future target - we are good to merge the build time class generation code (it is the only feature in this PR), right?

[GitHub] [cxf] rmannibucau commented on pull request #721: [cxf-8340] add precompiled class for Graalvm native support

2020-11-16 Thread GitBox
rmannibucau commented on pull request #721: URL: https://github.com/apache/cxf/pull/721#issuecomment-727945948 @reta well, I see two options: do another pre-721 PR with the archi refacto and then rebase 721 (but can be some work which will not bring much IMHO) or go with 721 as it. a Next

[GitHub] [cxf] rmannibucau commented on pull request #721: [cxf-8340] add precompiled class for Graalvm native support

2020-11-15 Thread GitBox
rmannibucau commented on pull request #721: URL: https://github.com/apache/cxf/pull/721#issuecomment-72755 @dufoli if you can, at least the classloader one is important. A doc word on how to capture the proxies at build time - even with a "main" and exec-maven-plugin if you don't want

[GitHub] [cxf] rmannibucau commented on pull request #721: [cxf-8340] add precompiled class for Graalvm native support

2020-11-15 Thread GitBox
rmannibucau commented on pull request #721: URL: https://github.com/apache/cxf/pull/721#issuecomment-727582333 2. yes but the classloader is not always the class classloader, it can be another one (more or less for the same reason you use a classloader to store proxies). Some servers

[GitHub] [cxf] rmannibucau commented on pull request #721: [cxf-8340] add precompiled class for Graalvm native support

2020-11-15 Thread GitBox
rmannibucau commented on pull request #721: URL: https://github.com/apache/cxf/pull/721#issuecomment-727577291 @dufoli 1. cause the parameters are not used anyway and I started to investigate if it was not the source of the stack error. Sure it can be in another patch (just

[GitHub] [cxf] rmannibucau commented on pull request #721: [cxf-8340] add precompiled class for Graalvm native support

2020-11-15 Thread GitBox
rmannibucau commented on pull request #721: URL: https://github.com/apache/cxf/pull/721#issuecomment-727572118 @dufoli congrats ;) Wonder if the static weak maps can't be dropped with something like: diff --git

[GitHub] [cxf] rmannibucau commented on pull request #721: [cxf-8340] add precompiled class for Graalvm native support

2020-11-15 Thread GitBox
rmannibucau commented on pull request #721: URL: https://github.com/apache/cxf/pull/721#issuecomment-727558031 @dufoli how do I reproduce the error? ran core asm tests and jaxb tests and both run on my computer. This is an

[GitHub] [cxf] rmannibucau commented on pull request #721: [cxf-8340] add precompiled class for Graalvm native support

2020-11-15 Thread GitBox
rmannibucau commented on pull request #721: URL: https://github.com/apache/cxf/pull/721#issuecomment-727541323 @dufoli when you do a classwriter.toByteArray() (like https://github.com/apache/cxf/pull/721/files#diff-0712fa792505d468314a10232664fa6b6d1c7cb1b58a8048b8310be1f677ae37R82) you

[GitHub] [cxf] rmannibucau commented on pull request #721: [cxf-8340] add precompiled class for Graalvm native support

2020-11-15 Thread GitBox
rmannibucau commented on pull request #721: URL: https://github.com/apache/cxf/pull/721#issuecomment-727536325 @dufoli if you can point out some code i can probably have a more precise look. Classes being unique per classloader it can have been a bug which was not preventing to work but

[GitHub] [cxf] rmannibucau commented on pull request #721: [cxf-8340] add precompiled class for Graalvm native support

2020-11-15 Thread GitBox
rmannibucau commented on pull request #721: URL: https://github.com/apache/cxf/pull/721#issuecomment-727532121 You can debug normally the tests (through an IDE). Now you can also dump the bytecode you generate in a /tmp/foo.class and open it with jd-gui or javap to check what was

[GitHub] [cxf] rmannibucau commented on pull request #721: [cxf-8340] add precompiled class for Graalvm native support

2020-11-14 Thread GitBox
rmannibucau commented on pull request #721: URL: https://github.com/apache/cxf/pull/721#issuecomment-727240024 @dufoli the bug refacto is to encapsulate asm, not this PR ;). IMHO adding such a method was a blocker because it was only needed for your project and was not achieving the goal

[GitHub] [cxf] rmannibucau commented on pull request #721: [cxf-8340] add precompiled class for Graalvm native support

2020-11-14 Thread GitBox
rmannibucau commented on pull request #721: URL: https://github.com/apache/cxf/pull/721#issuecomment-727227166 @dufoli guess this class can stay static since it is a list of constants - I suspect a clean refacto would be to encapsulate all these internals but this is way too much for this