Re: Source location static inlining

I am against polluting (actually, almost doubling!) our public API just because we couldn't figure out how to weave bytecode in the way we want. For the records, specialization will be inevitable for transforming SLF4J and Log4j 1 calls. On Tue, Jul 12, 2022 at 10:49 PM Piotr P. Karwasz wrote:

Re: Source location static inlining

Hi Volkan, > My next step will be to run this at compile time. (Will keep you posted.) > > Please note the specialization on `Logger#info(String)` call in > `AppTransforming`. Since Ralph did not like thread-locals, we need to > implement every such possible specialization of the public API. I th

Re: Source location static inlining

This AppTransforming class , transforms the following private static final Logger LOGGER = LogManager.getLogger(); public static void main(String[] args) { LOGGER.info("should log at

Re: Source location static inlining

> On Jul 10, 2022, at 9:36 PM, Piotr P. Karwasz wrote: > > Hi Volkan, > > On Sun, 10 Jul 2022 at 21:47, Volkan Yazıcı wrote: >> Piotr, what is your take on my claim that this optimization won't work for >> bridges (SLF4J, log4j-1.2-api, etc.)? > > If you are using a `ThreadLocal` to store t

Re: Source location static inlining

Hi Volkan, On Sun, 10 Jul 2022 at 21:47, Volkan Yazıcı wrote: > Piotr, what is your take on my claim that this optimization won't work for > bridges (SLF4J, log4j-1.2-api, etc.)? If you are using a `ThreadLocal` to store the location information just before a logging call, you can use it before

Re: Source location static inlining

I should note that It would of course be possible to support using our own @Log4j2 annotation and support Lombok’s annotation if it is present, but if we do not require Lombok we are almost certainly going to end up creating our own version of https://github.com/projectlombok/lombok/blob/master

Re: Source location static inlining

Yes, I read the code and I don’t like it. There is no reason to create the ThreadLocal. You have some magic code that isn’t demonstrated in your POC to create a StackTraceElement. That is actually the hard part. Setting it into a ThreadLocal so it can be used later will work but really doesn’

Re: Source location static inlining

Might I ask why? lots of people use it and adding the ability to enhance the logging just by adding a dependency seems like a win to me. Ralph > On Jul 10, 2022, at 2:27 PM, Volkan Yazıcı wrote: > > -1 from me for a Log4j functionality depending on Lombok. > >> On Sun, Jul 10, 2022 at 11:17

Re: Source location static inlining

-1 from me for a Log4j functionality depending on Lombok. On Sun, Jul 10, 2022 at 11:17 PM Ralph Goers wrote: > https://www.baeldung.com/lombok-custom-annotation seems to imply that we > could > add this support by extending Lombok. > > Ralph > > > On Jul 10, 2022, at 1:58 PM, Ralph Goers > wro

Re: Source location static inlining

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

Re: Source location static inlining

On Sun, Jul 10, 2022 at 4:58 PM Ralph Goers 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

Re: Source location static inlining

https://www.baeldung.com/lombok-custom-annotation seems to imply that we could add this support by extending Lombok. Ralph > On Jul 10, 2022, at 1:58 PM, Ralph Goers wrote: > > Because I don’t see how it can work. > > A ThreadLocal is a run time construct. If you are saving location informat

Re: Source location static inlining

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

Re: Source location static inlining

Why do you prefer `withLocation()` compared to the thread-local approach? On Sun, Jul 10, 2022 at 9:55 PM Ralph Goers 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 > va

Re: Source location static inlining

On Sat, Jul 9, 2022 at 8:16 PM Ralph Goers wrote: > 1. I’d like to not have to generate new source code for every class doing > logging. > I am not sure how Lombok does it but I will want to investigate that. > I have studied (at least tried to!) Lombok sources a bit... They are pretty arcane. N

Re: Source location static inlining

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

Re: Source location static inlining

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

Re: Source location static inlining

> On Jul 9, 2022, at 11:16 AM, Ralph Goers wrote: > > I’ve been thinking about this a lot. There are a few issues. > > 1. I’d like to not have to generate new source code for every class doing > logging. > I am not sure how Lombok does it but I will want to investigate that. > 2. Your solut

Re: Source location static inlining

I’ve been thinking about this a lot. There are a few issues. 1. I’d like to not have to generate new source code for every class doing logging. I am not sure how Lombok does it but I will want to investigate that. 2. Your solution looks like it is still done at runtime. I would like to do it a

Re: Source location static inlining

Hi Volkan, On Sat, 9 Jul 2022 at 12:05, Volkan Yazıcı 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 g

Re: Source location static inlining

It would be truly awesome to get this to work! Triple yay! I would keep it as simple as possible though, and not worry too much about being super user friendly. Asking folks to recompile to get this feature is not a big ask, and avoiding complexity like weaving is a big advantage. Not working for

Re: Source location static inlining

There might be something to reuse from https://projectlombok.org/ Gary On Sat, Jul 9, 2022, 06:05 Volkan Yazıcı wrote: > Inspired by this SO post > and > with some help from Piotr , > I > have drafted a

Source location static inlining

Inspired by this SO post and with some help from Piotr , I have drafted an example where I redefine a class such that every logger call is preceded with a static source location capture. The experiment aims