Ralph, have you read the sources I have shared in the email? The only runtime cost in my example was a thread-local set and get. I use the TL as an indirect comm. medium to pass the source location information to the logger and it works. Given you said "I don’t see how it can work", can you give an example where it doesn't work?
My example was a POC for demonstrating the feasibility of the approach and there I have used the generated bytecode at runtime. The difficult part is generating the bytecode (which I already did), how one employs that, either at runtime or compile time, is a simpler implementation detail. On Sun, Jul 10, 2022 at 10:58 PM Ralph Goers <ralph.go...@dslextreme.com> wrote: > Because I don’t see how it can work. > > A ThreadLocal is a run time construct. If you are saving location > information in > a ThreadLocal then it is only good for the life of the request and/or > thread. But > the location information will never change. It is fixed even before the > Class is loaded. > So I simply don’t see the point of using a ThreadLocal. > > Furthermore, I am ONLY interested in injecting location information at > compile > time. That means there will always be zero run time cost. Using an agent > or some > other technique requires modifying the class as it is loaded or other > runtime > manipulations. > > I’ve looked at the HandleLog method in Lombok. It actually isn’t very > complicated. > It uses JCTree and the util package from javac and adds the logger field > directly > to the class the compiler is constructing. From what I can tell it should > also be > possible to scan the source and modify the appropriate lines in the same > way, > but it would require a POC. > > Ralph > > > On Jul 10, 2022, at 1:09 PM, Volkan Yazıcı <vol...@yazi.ci> wrote: > > > > Why do you prefer `withLocation()` compared to the thread-local approach? > > > > On Sun, Jul 10, 2022 at 9:55 PM Ralph Goers <ralph.go...@dslextreme.com> > > wrote: > > > >> This is exactly why I believe we should only support LogBuilder as it > >> already > >> has withLocation(). I see no point in adding the parameter to all the > >> non-fluent > >> variations. > >> > >> To work, the location must be passed as a parameter on the logging API > >> call. > >> Thus it won’t work for Log4j 1 or SLF4J. > >> > >> Ralph > >> > >>> On Jul 10, 2022, at 12:47 PM, Volkan Yazıcı <vol...@yazi.ci> wrote: > >>> > >>> Indeed supporting both static and dynamic weaving seems like the ideal > >>> approach. (SPI-Fly is an interesting one. My OSGi illiteracy blocked me > >>> from wrapping my mind around all of its details. Nevertheless, I think > I > >>> get the gist of it.) For both functionalities, we need to receive a > >> package > >>> list to scan for, right? > >>> > >>> Translating logger calls to the ones that receive the source location > >>> information as arguments is also a valid direction. Though note that > this > >>> requires doubling the size of the API surface, AFAIC. That is, for > every > >>> `info(String)`, we need to introduce `info(String, SourceLocation)`, > etc. > >>> Hence, I am inclined to go this route unless I am missing something. > >>> > >>> Piotr, what is your take on my claim that this optimization won't work > >> for > >>> bridges (SLF4J, log4j-1.2-api, etc.)? > >>> > >>> On Sat, Jul 9, 2022 at 5:02 PM Piotr P. Karwasz < > piotr.karw...@gmail.com > >>> > >>> wrote: > >>> > >>>> Hi Volkan, > >>>> > >>>> On Sat, 9 Jul 2022 at 12:05, Volkan Yazıcı <vol...@yazi.ci> wrote: > >>>>> I think we can extend this experiment to implement zero-cost source > >>>>> location capture for Log4j. Though I will appreciate your help on > some > >>>>> loose ends. Assuming we have a bullet-proof mechanism to inline > source > >>>>> location capture given a class, what is the right way to ship this? > As > >> a > >>>>> Maven plugin that kicks in at compile time? Wouldn't that make this > >>>> feature > >>>>> impossible to use without recompiling user sources? As a runtime > >> utility? > >>>>> If so, what about the cost of classpath scanning & weaving? If the > >>>> bytecode > >>>>> weaving only intercepts at Log4j API calls, this won't work for > Log4j 1 > >>>>> bridge, SLF4J, or any other indirect access to the Log4j API. What do > >> you > >>>>> think? I have used a thread-local to pass the source location to the > >>>>> caller, is there a better alternative? (Putting aside the > >> dynamic-scoped > >>>>> variables to be shipped with Loom.) > >>>> > >>>> Great idea. I think that we can provide both a static and dynamic > >>>> weaver from the same code (cf. SPI-Fly: > >>>> https://github.com/apache/aries/tree/trunk/spi-fly). Developers would > >>>> be advised to statically weave their artifacts, while system > >>>> administrators could do it during runtime. > >>>> > >>>> The usage of a `ThreadLocal` seems Ok to me. Alternatively we could > >>>> add some parameters to the `Logger.log` calls, but this would mean 4 > >>>> additional parameters on each simple call and we'll end up using the > >>>> `Logger.log` method with an Object array. > >>>> > >>>> Piotr > >>>> > >> > >> > >
