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 >>>> >> >>
