Now the PR is merged and seems we have consensus module attention to possible performance issues when integrated so I will start the vote.
On Sat, May 15, 2021 at 3:53 AM Reuven Lax <re...@google.com> wrote: > Microbenchmarks are tough for these benchmarks. In the past, we've had > changes that increase the time it took to generate bytcode. While this his > minimal impact on real pipelines (since bytecode is generated on worker > startup), it has an outsized impact on microbencmark run time. > > On Wed, May 12, 2021 at 5:55 AM Ismaël Mejía <ieme...@gmail.com> wrote: > >> Testing this particular kind of PR for perf would be tricky, I think the >> easiest thing we can notice is if the runtime of the CI tests differs a lot. >> I really don't think the generated bytecode with the new version would >> differ much but is for sure something we should pay attention to. >> And worse case scenario reversing the upgrade should not be that >> difficult given Beam's well confined dependency on bytebuddy. >> >> Other ideas/comments? >> >> >> On Mon, May 10, 2021 at 7:16 PM Reuven Lax <re...@google.com> wrote: >> >>> What's the best way to test a PR for perf? >>> >>> On Mon, May 10, 2021 at 8:59 AM Kenneth Knowles <k...@apache.org> wrote: >>> >>>> If nothing breaks, and we check perf, then absolutely this seems good. >>>> >>>> Kenn >>>> >>>> On Mon, May 10, 2021 at 12:38 AM Ismaël Mejía <ieme...@gmail.com> >>>> wrote: >>>> >>>>> Most issues on the previous migration were related to changes on >>>>> behavior of class-loading on Java 11. It seems Oracle is taking a more >>>>> backwards compatible on latest releases, so let's hope everything will go >>>>> well. In the meantime I tested the upgrade locally and tests are passing >>>>> ok >>>>> so we should be good to go. I opened a PR [1] for the version upgrade and >>>>> assuming consensus on this proposal I expect we can pass to vote soon. >>>>> >>>>> [1] https://github.com/apache/beam/pull/14766 >>>>> >>>>> >>>>> On Sun, May 9, 2021 at 6:13 PM Reuven Lax <re...@google.com> wrote: >>>>> >>>>>> We've had some issues in the past with semantic changes in ByteBuddy >>>>>> (I think related to new Java versions) that required rewriting code in >>>>>> Beam. >>>>>> >>>>>> On Sat, May 8, 2021 at 10:46 PM Ismaël Mejía <ieme...@gmail.com> >>>>>> wrote: >>>>>> >>>>>>> What were the issues last time Reuven? I remember that the release >>>>>>> and upgrade PR were pretty smooth, were there unintended consequences >>>>>>> from >>>>>>> the library changes themselves? >>>>>>> >>>>>>> >>>>>>> On Sun, May 9, 2021 at 12:36 AM Reuven Lax <re...@google.com> wrote: >>>>>>> >>>>>>>> Sounds good. Based on previous experience though, this might be a >>>>>>>> difficult upgrade to do. >>>>>>>> >>>>>>>> On Sat, May 8, 2021 at 12:57 AM Ismaël Mejía <ieme...@gmail.com> >>>>>>>> wrote: >>>>>>>> >>>>>>>>> The version of bytebuddy Beam is vendoring (1.10.8) is already 16 >>>>>>>>> months old and >>>>>>>>> it is not compatible with more recent versions of Java. I would >>>>>>>>> like to propose >>>>>>>>> that we upgrade it [1] to the most recent version (1.11.0) [2] so >>>>>>>>> we can benefit >>>>>>>>> of the latest improvements for Java 16/17 and upgraded ASM. >>>>>>>>> >>>>>>>>> If everyone agrees I would like to volunteer as the release >>>>>>>>> manager for this >>>>>>>>> upgrade. >>>>>>>>> >>>>>>>>> [1] https://issues.apache.org/jira/browse/BEAM-12241 >>>>>>>>> [2] >>>>>>>>> https://github.com/raphw/byte-buddy/blob/master/release-notes.md >>>>>>>>> >>>>>>>>>
