You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@logging.apache.org by Volkan Yazıcı <vo...@yazi.ci> on 2022/07/09 10:04:51 UTC

Source location static inlining

Inspired by this SO post <https://stackoverflow.com/a/72437386/1278899> and
with some help from Piotr <https://stackoverflow.com/a/72916795/1278899>, 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
to replace the current source location capture that uses an exception-based
expensive mechanism with inlining the source location using bytecode
weaving. The sources are publicly available on GitHub.
<https://github.com/vy/asm-playground/tree/master/src/main/java/com/vlkan>
In a nutshell, the magic is as follows:

I have a logger library (Log4j.java
<https://github.com/vy/asm-playground/blob/master/src/main/java/com/vlkan/Log4j.java>)
as follows:

    public static final ThreadLocal<SourceLocation> LOCATION_REF =
ThreadLocal.withInitial(SourceLocation::new);

    public static void log() {
        SourceLocation location = LOCATION_REF.get();
        boolean locationProvided = location.lineNumber > 0;
        if (!locationProvided) {
            StackTraceElement[] stackTraceElements = new
Throwable().getStackTrace();
            // Skip the first element pointing to this method.
            StackTraceElement stackTraceElement = stackTraceElements[1];
            location.init(
                    stackTraceElement.getFileName(),
                    stackTraceElement.getClassName(),
                    stackTraceElement.getMethodName(),
                    stackTraceElement.getLineNumber());
        }
        System.out.format(
                "[%s] %s%n",
                location,
                locationProvided ? "provided location" : "populated
location");
    }

Here note how `log()` uses a thread-local to see if there is already a
`SourceLocation` provided. If so, it leverages that, otherwise it populates
the source location using the stack trace of an exception.

Below is my actual application (AppActual.java
<https://github.com/vy/asm-playground/blob/master/src/main/java/com/vlkan/AppActual.java>),
that is, what the actual/existing user code looks like:

    public static void main(String[] args) {
        System.out.println("should log at line 9");
        log();
        System.out.println("nothing to see here");
        System.out.println("should log at line 12");
        log();
        f();
    }

    private static void f() {
        System.out.println("adding some indirection");
        System.out.println("should log at line 19");
        log();
    }

I want to transform this into the following expected form (AppExpected.java
<https://github.com/vy/asm-playground/blob/master/src/main/java/com/vlkan/AppExpected.java>)
that exploits the `LOCATION_REF` thread-local to inline the source location
information:

    public static void main(String[] args) {
        System.out.println("should log at line 9");
        LOCATION_REF.get().init("AppExpected.java",
"com.vlkan.AppExpected", "main", 9);
        log();
        System.out.println("nothing to see here");
        System.out.println("should log at line 12");
        LOCATION_REF.get().init("AppExpected.java",
"com.vlkan.AppExpected", "main", 12);
        log();
        f();
    }

    private static void f() {
        System.out.println("adding some indirection");
        System.out.println("should log at line 19");
        LOCATION_REF.get().init("AppExpected.java",
"com.vlkan.AppExpected", "main", 19);
        log();
    }

And... 👉 AppTransforming.java
<https://github.com/vy/asm-playground/blob/master/src/main/java/com/vlkan/AppTransforming.java>
👈, my dear friends, performs exactly this transformation: converts
`AppActual` bytecode into `AppExpected`. 😍

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

Re: Source location static inlining

Posted by Volkan Yazıcı <vo...@yazi.ci>.
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 <pi...@gmail.com>
wrote:

> 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 think we just need a helper class that has a static method for each
> method of the `Logger` class. E.g. for `Logger#info(String)` we can
> have `LoggerUtil#info(Logger, String, int)`. Since the signatures are
> almost identical we just need to push the location index onto the
> stack and replace the call of one method with the other. Since the
> util methods will be very short, the JIT compiler will end up inlining
> them and we don't have to play with the order of the arguments at a
> bytecode level.
>
> I have a working prototype here:
> https://github.com/ppkarwasz/asm-playground
>
> The main problem with the prototype is that it adds a static field to
> the woven class, but that can be corrected by either generating an
> internal class or using an external registry like you do.
>
> Piotr
>

Re: Source location static inlining

Posted by "Piotr P. Karwasz" <pi...@gmail.com>.
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 think we just need a helper class that has a static method for each
method of the `Logger` class. E.g. for `Logger#info(String)` we can
have `LoggerUtil#info(Logger, String, int)`. Since the signatures are
almost identical we just need to push the location index onto the
stack and replace the call of one method with the other. Since the
util methods will be very short, the JIT compiler will end up inlining
them and we don't have to play with the order of the arguments at a
bytecode level.

I have a working prototype here: https://github.com/ppkarwasz/asm-playground

The main problem with the prototype is that it adds a static field to
the woven class, but that can be corrected by either generating an
internal class or using an external registry like you do.

Piotr

Re: Source location static inlining

Posted by Volkan Yazıcı <vo...@yazi.ci>.
This AppTransforming class
<https://github.com/vy/asm-playground/blob/master/src/main/java/com/vlkan/v2/AppTransforming.java>,
transforms the following

    private static final Logger LOGGER = LogManager.getLogger();

    public static void main(String[] args) {
        LOGGER.info("should log at line 11");
        System.out.println("nothing to see here");
        LOGGER.info("should log at line 13");
        f();
    }

    private static void f() {
        System.out.println("adding some indirection");
        LOGGER.info("should log at line 19");
    }

to this:

    private static final Logger LOGGER = LogManager.getLogger();

    public static void main(String[] args) {

LOGGER.atInfo().withLocation(Log4jLocationRegistry.get(0)).log("should log
at line 11");
        System.out.println("nothing to see here");

LOGGER.atInfo().withLocation(Log4jLocationRegistry.get(1)).log("should log
at line 13");
        f();
    }

    private static void f() {
        System.out.println("adding some indirection");

LOGGER.atInfo().withLocation(Log4jLocationRegistry.get(2)).log("should log
at line 19");
    }

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.

On Sat, Jul 9, 2022 at 12:04 PM Volkan Yazıcı <vo...@yazi.ci> wrote:

> Inspired by this SO post <https://stackoverflow.com/a/72437386/1278899>
> and with some help from Piotr
> <https://stackoverflow.com/a/72916795/1278899>, 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 to replace the current
> source location capture that uses an exception-based expensive mechanism
> with inlining the source location using bytecode weaving. The sources are
> publicly available on GitHub.
> <https://github.com/vy/asm-playground/tree/master/src/main/java/com/vlkan>
> In a nutshell, the magic is as follows:
>
> I have a logger library (Log4j.java
> <https://github.com/vy/asm-playground/blob/master/src/main/java/com/vlkan/Log4j.java>)
> as follows:
>
>     public static final ThreadLocal<SourceLocation> LOCATION_REF =
> ThreadLocal.withInitial(SourceLocation::new);
>
>     public static void log() {
>         SourceLocation location = LOCATION_REF.get();
>         boolean locationProvided = location.lineNumber > 0;
>         if (!locationProvided) {
>             StackTraceElement[] stackTraceElements = new
> Throwable().getStackTrace();
>             // Skip the first element pointing to this method.
>             StackTraceElement stackTraceElement = stackTraceElements[1];
>             location.init(
>                     stackTraceElement.getFileName(),
>                     stackTraceElement.getClassName(),
>                     stackTraceElement.getMethodName(),
>                     stackTraceElement.getLineNumber());
>         }
>         System.out.format(
>                 "[%s] %s%n",
>                 location,
>                 locationProvided ? "provided location" : "populated
> location");
>     }
>
> Here note how `log()` uses a thread-local to see if there is already a
> `SourceLocation` provided. If so, it leverages that, otherwise it populates
> the source location using the stack trace of an exception.
>
> Below is my actual application (AppActual.java
> <https://github.com/vy/asm-playground/blob/master/src/main/java/com/vlkan/AppActual.java>),
> that is, what the actual/existing user code looks like:
>
>     public static void main(String[] args) {
>         System.out.println("should log at line 9");
>         log();
>         System.out.println("nothing to see here");
>         System.out.println("should log at line 12");
>         log();
>         f();
>     }
>
>     private static void f() {
>         System.out.println("adding some indirection");
>         System.out.println("should log at line 19");
>         log();
>     }
>
> I want to transform this into the following expected form (
> AppExpected.java
> <https://github.com/vy/asm-playground/blob/master/src/main/java/com/vlkan/AppExpected.java>)
> that exploits the `LOCATION_REF` thread-local to inline the source location
> information:
>
>     public static void main(String[] args) {
>         System.out.println("should log at line 9");
>         LOCATION_REF.get().init("AppExpected.java",
> "com.vlkan.AppExpected", "main", 9);
>         log();
>         System.out.println("nothing to see here");
>         System.out.println("should log at line 12");
>         LOCATION_REF.get().init("AppExpected.java",
> "com.vlkan.AppExpected", "main", 12);
>         log();
>         f();
>     }
>
>     private static void f() {
>         System.out.println("adding some indirection");
>         System.out.println("should log at line 19");
>         LOCATION_REF.get().init("AppExpected.java",
> "com.vlkan.AppExpected", "main", 19);
>         log();
>     }
>
> And... 👉 AppTransforming.java
> <https://github.com/vy/asm-playground/blob/master/src/main/java/com/vlkan/AppTransforming.java>
> 👈, my dear friends, performs exactly this transformation: converts
> `AppActual` bytecode into `AppExpected`. 😍
>
> 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.)
>

Re: Source location static inlining

Posted by Remko Popma <re...@gmail.com>.
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 Log4j 1 etc is also not a problem, and may even further our
goal to get people to migrate.

On Sat, Jul 9, 2022 at 19:39 Gary Gregory <ga...@gmail.com> wrote:

> There might be something to reuse from https://projectlombok.org/
>
> Gary
>
> On Sat, Jul 9, 2022, 06:05 Volkan Yazıcı <vo...@yazi.ci> wrote:
>
> > Inspired by this SO post <https://stackoverflow.com/a/72437386/1278899>
> > and
> > with some help from Piotr <https://stackoverflow.com/a/72916795/1278899
> >,
> > 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
> > to replace the current source location capture that uses an
> exception-based
> > expensive mechanism with inlining the source location using bytecode
> > weaving. The sources are publicly available on GitHub.
> > <
> https://github.com/vy/asm-playground/tree/master/src/main/java/com/vlkan>
> > In a nutshell, the magic is as follows:
> >
> > I have a logger library (Log4j.java
> > <
> >
> https://github.com/vy/asm-playground/blob/master/src/main/java/com/vlkan/Log4j.java
> > >)
> > as follows:
> >
> >     public static final ThreadLocal<SourceLocation> LOCATION_REF =
> > ThreadLocal.withInitial(SourceLocation::new);
> >
> >     public static void log() {
> >         SourceLocation location = LOCATION_REF.get();
> >         boolean locationProvided = location.lineNumber > 0;
> >         if (!locationProvided) {
> >             StackTraceElement[] stackTraceElements = new
> > Throwable().getStackTrace();
> >             // Skip the first element pointing to this method.
> >             StackTraceElement stackTraceElement = stackTraceElements[1];
> >             location.init(
> >                     stackTraceElement.getFileName(),
> >                     stackTraceElement.getClassName(),
> >                     stackTraceElement.getMethodName(),
> >                     stackTraceElement.getLineNumber());
> >         }
> >         System.out.format(
> >                 "[%s] %s%n",
> >                 location,
> >                 locationProvided ? "provided location" : "populated
> > location");
> >     }
> >
> > Here note how `log()` uses a thread-local to see if there is already a
> > `SourceLocation` provided. If so, it leverages that, otherwise it
> populates
> > the source location using the stack trace of an exception.
> >
> > Below is my actual application (AppActual.java
> > <
> >
> https://github.com/vy/asm-playground/blob/master/src/main/java/com/vlkan/AppActual.java
> > >),
> > that is, what the actual/existing user code looks like:
> >
> >     public static void main(String[] args) {
> >         System.out.println("should log at line 9");
> >         log();
> >         System.out.println("nothing to see here");
> >         System.out.println("should log at line 12");
> >         log();
> >         f();
> >     }
> >
> >     private static void f() {
> >         System.out.println("adding some indirection");
> >         System.out.println("should log at line 19");
> >         log();
> >     }
> >
> > I want to transform this into the following expected form
> (AppExpected.java
> > <
> >
> https://github.com/vy/asm-playground/blob/master/src/main/java/com/vlkan/AppExpected.java
> > >)
> > that exploits the `LOCATION_REF` thread-local to inline the source
> location
> > information:
> >
> >     public static void main(String[] args) {
> >         System.out.println("should log at line 9");
> >         LOCATION_REF.get().init("AppExpected.java",
> > "com.vlkan.AppExpected", "main", 9);
> >         log();
> >         System.out.println("nothing to see here");
> >         System.out.println("should log at line 12");
> >         LOCATION_REF.get().init("AppExpected.java",
> > "com.vlkan.AppExpected", "main", 12);
> >         log();
> >         f();
> >     }
> >
> >     private static void f() {
> >         System.out.println("adding some indirection");
> >         System.out.println("should log at line 19");
> >         LOCATION_REF.get().init("AppExpected.java",
> > "com.vlkan.AppExpected", "main", 19);
> >         log();
> >     }
> >
> > And... 👉 AppTransforming.java
> > <
> >
> https://github.com/vy/asm-playground/blob/master/src/main/java/com/vlkan/AppTransforming.java
> > >
> > 👈, my dear friends, performs exactly this transformation: converts
> > `AppActual` bytecode into `AppExpected`. 😍
> >
> > 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.)
> >
>

Re: Source location static inlining

Posted by Gary Gregory <ga...@gmail.com>.
There might be something to reuse from https://projectlombok.org/

Gary

On Sat, Jul 9, 2022, 06:05 Volkan Yazıcı <vo...@yazi.ci> wrote:

> Inspired by this SO post <https://stackoverflow.com/a/72437386/1278899>
> and
> with some help from Piotr <https://stackoverflow.com/a/72916795/1278899>,
> 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
> to replace the current source location capture that uses an exception-based
> expensive mechanism with inlining the source location using bytecode
> weaving. The sources are publicly available on GitHub.
> <https://github.com/vy/asm-playground/tree/master/src/main/java/com/vlkan>
> In a nutshell, the magic is as follows:
>
> I have a logger library (Log4j.java
> <
> https://github.com/vy/asm-playground/blob/master/src/main/java/com/vlkan/Log4j.java
> >)
> as follows:
>
>     public static final ThreadLocal<SourceLocation> LOCATION_REF =
> ThreadLocal.withInitial(SourceLocation::new);
>
>     public static void log() {
>         SourceLocation location = LOCATION_REF.get();
>         boolean locationProvided = location.lineNumber > 0;
>         if (!locationProvided) {
>             StackTraceElement[] stackTraceElements = new
> Throwable().getStackTrace();
>             // Skip the first element pointing to this method.
>             StackTraceElement stackTraceElement = stackTraceElements[1];
>             location.init(
>                     stackTraceElement.getFileName(),
>                     stackTraceElement.getClassName(),
>                     stackTraceElement.getMethodName(),
>                     stackTraceElement.getLineNumber());
>         }
>         System.out.format(
>                 "[%s] %s%n",
>                 location,
>                 locationProvided ? "provided location" : "populated
> location");
>     }
>
> Here note how `log()` uses a thread-local to see if there is already a
> `SourceLocation` provided. If so, it leverages that, otherwise it populates
> the source location using the stack trace of an exception.
>
> Below is my actual application (AppActual.java
> <
> https://github.com/vy/asm-playground/blob/master/src/main/java/com/vlkan/AppActual.java
> >),
> that is, what the actual/existing user code looks like:
>
>     public static void main(String[] args) {
>         System.out.println("should log at line 9");
>         log();
>         System.out.println("nothing to see here");
>         System.out.println("should log at line 12");
>         log();
>         f();
>     }
>
>     private static void f() {
>         System.out.println("adding some indirection");
>         System.out.println("should log at line 19");
>         log();
>     }
>
> I want to transform this into the following expected form (AppExpected.java
> <
> https://github.com/vy/asm-playground/blob/master/src/main/java/com/vlkan/AppExpected.java
> >)
> that exploits the `LOCATION_REF` thread-local to inline the source location
> information:
>
>     public static void main(String[] args) {
>         System.out.println("should log at line 9");
>         LOCATION_REF.get().init("AppExpected.java",
> "com.vlkan.AppExpected", "main", 9);
>         log();
>         System.out.println("nothing to see here");
>         System.out.println("should log at line 12");
>         LOCATION_REF.get().init("AppExpected.java",
> "com.vlkan.AppExpected", "main", 12);
>         log();
>         f();
>     }
>
>     private static void f() {
>         System.out.println("adding some indirection");
>         System.out.println("should log at line 19");
>         LOCATION_REF.get().init("AppExpected.java",
> "com.vlkan.AppExpected", "main", 19);
>         log();
>     }
>
> And... 👉 AppTransforming.java
> <
> https://github.com/vy/asm-playground/blob/master/src/main/java/com/vlkan/AppTransforming.java
> >
> 👈, my dear friends, performs exactly this transformation: converts
> `AppActual` bytecode into `AppExpected`. 😍
>
> 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.)
>

Re: Source location static inlining

Posted by Ralph Goers <ra...@dslextreme.com>.

> On Jul 10, 2022, at 9:36 PM, Piotr P. Karwasz <pi...@gmail.com> wrote:
> 
> Hi Volkan,
> 
> On Sun, 10 Jul 2022 at 21:47, Volkan Yazıcı <vo...@yazi.ci> 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 every Log4j 1.x and
> SLF4J call too. So it should work for every API that has a bridge to
> Log4j2.
> 
> However, if we are weaving anyway, we could as well replace all
> references to Log4j 1.x and SLF4J with direct Log4j2 calls. For old
> EOL libraries using Log4j 1.x this could be quite popular.


I love this! It is essentially the same thing I am proposing but looking for 
other API calls as well as those made to Log4j 2.

Ralph

Re: Source location static inlining

Posted by "Piotr P. Karwasz" <pi...@gmail.com>.
Hi Volkan,

On Sun, 10 Jul 2022 at 21:47, Volkan Yazıcı <vo...@yazi.ci> 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 every Log4j 1.x and
SLF4J call too. So it should work for every API that has a bridge to
Log4j2.

However, if we are weaving anyway, we could as well replace all
references to Log4j 1.x and SLF4J with direct Log4j2 calls. For old
EOL libraries using Log4j 1.x this could be quite popular.

Piotr

Re: Source location static inlining

Posted by Ralph Goers <ra...@dslextreme.com>.
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’t seem worth 
the trouble. You already have to do manipulation of the Class to insert the 
ThreadLocal update before every logging call. That hardly seems worth it 
when it is just as easy to convert every logging call into its fluent form.

Note also that your example creates a new StackTraceElement each time 
the location is referenced instead of reusing a statically declared one.

As I’ve said several times now, the proper solution is to determine the location 
information at compile time. It would seem we both agree on that. I would 
store them in a Map or an array or even generate each as a separate Field 
in the class. I would then replace every logging method with the LogBuilder 
call to pass the location. 

It would seem that we agree on most of the mechanics except you would 
use a ThreadLocal AND do byte code manipulation whereas I would rely 
on byte code manipulation alone.

Ralph


> On Jul 10, 2022, at 2:26 PM, Volkan Yazıcı <vo...@yazi.ci> wrote:
> 
> 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 <ra...@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ı <vo...@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 <ra...@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ı <vo...@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.karwasz@gmail.com
>>>>> 
>>>>> wrote:
>>>>> 
>>>>>> Hi Volkan,
>>>>>> 
>>>>>> On Sat, 9 Jul 2022 at 12:05, Volkan Yazıcı <vo...@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
>>>>>> 
>>>> 
>>>> 
>> 
>> 


Re: Source location static inlining

Posted by Volkan Yazıcı <vo...@yazi.ci>.
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 <ra...@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ı <vo...@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 <ra...@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ı <vo...@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.karwasz@gmail.com
> >>>
> >>> wrote:
> >>>
> >>>> Hi Volkan,
> >>>>
> >>>> On Sat, 9 Jul 2022 at 12:05, Volkan Yazıcı <vo...@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
> >>>>
> >>
> >>
>
>

Re: Source location static inlining

Posted by Ralph Goers <ra...@dslextreme.com>.
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/src/core/lombok/javac/handlers/JavacHandlerUtil.java <https://github.com/projectlombok/lombok/blob/master/src/core/lombok/javac/handlers/JavacHandlerUtil.java> 
as that is where a lot of the real work happens.

Ralph

> On Jul 10, 2022, at 3:32 PM, Ralph Goers <ra...@dslextreme.com> wrote:
> 
> 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ı <vo...@yazi.ci> wrote:
>> 
>> -1 from me for a Log4j functionality depending on Lombok.
>> 
>>> On Sun, Jul 10, 2022 at 11:17 PM Ralph Goers <ra...@dslextreme.com>
>>> 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 <ra...@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ı <vo...@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.goers@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ı <vo...@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.karwasz@gmail.com
>>>>>>> 
>>>>>>> wrote:
>>>>>>> 
>>>>>>>> Hi Volkan,
>>>>>>>> 
>>>>>>>> On Sat, 9 Jul 2022 at 12:05, Volkan Yazıcı <vo...@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
>>>>>>>> 
>>>>>> 
>>>>>> 
>>>> 
>>> 
>>> 
> 


Re: Source location static inlining

Posted by Ralph Goers <ra...@dslextreme.com>.
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ı <vo...@yazi.ci> wrote:
> 
> -1 from me for a Log4j functionality depending on Lombok.
> 
>> On Sun, Jul 10, 2022 at 11:17 PM Ralph Goers <ra...@dslextreme.com>
>> 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 <ra...@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ı <vo...@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.goers@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ı <vo...@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.karwasz@gmail.com
>>>>>> 
>>>>>> wrote:
>>>>>> 
>>>>>>> Hi Volkan,
>>>>>>> 
>>>>>>> On Sat, 9 Jul 2022 at 12:05, Volkan Yazıcı <vo...@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
>>>>>>> 
>>>>> 
>>>>> 
>>> 
>> 
>> 


Re: Source location static inlining

Posted by Volkan Yazıcı <vo...@yazi.ci>.
-1 from me for a Log4j functionality depending on Lombok.

On Sun, Jul 10, 2022 at 11:17 PM Ralph Goers <ra...@dslextreme.com>
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 <ra...@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ı <vo...@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.goers@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ı <vo...@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.karwasz@gmail.com
> >>>>
> >>>> wrote:
> >>>>
> >>>>> Hi Volkan,
> >>>>>
> >>>>> On Sat, 9 Jul 2022 at 12:05, Volkan Yazıcı <vo...@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
> >>>>>
> >>>
> >>>
> >
>
>

Re: Source location static inlining

Posted by Ralph Goers <ra...@dslextreme.com>.
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 <ra...@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ı <vo...@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 <ra...@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ı <vo...@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.karwasz@gmail.com
>>>> 
>>>> wrote:
>>>> 
>>>>> Hi Volkan,
>>>>> 
>>>>> On Sat, 9 Jul 2022 at 12:05, Volkan Yazıcı <vo...@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
>>>>> 
>>> 
>>> 
> 


Re: Source location static inlining

Posted by Gary Gregory <ga...@gmail.com>.
On Sun, Jul 10, 2022 at 4:58 PM Ralph Goers <ra...@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 agree fully that it would be ideal to have this be a compile-time
transformation.

Gary

>
> 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ı <vo...@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 <ra...@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ı <vo...@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.karwasz@gmail.com
> >>>
> >>> wrote:
> >>>
> >>>> Hi Volkan,
> >>>>
> >>>> On Sat, 9 Jul 2022 at 12:05, Volkan Yazıcı <vo...@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
> >>>>
> >>
> >>
>

Re: Source location static inlining

Posted by Ralph Goers <ra...@dslextreme.com>.
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ı <vo...@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 <ra...@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ı <vo...@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.karwasz@gmail.com
>>> 
>>> wrote:
>>> 
>>>> Hi Volkan,
>>>> 
>>>> On Sat, 9 Jul 2022 at 12:05, Volkan Yazıcı <vo...@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
>>>> 
>> 
>> 


Re: Source location static inlining

Posted by Volkan Yazıcı <vo...@yazi.ci>.
Why do you prefer `withLocation()` compared to the thread-local approach?

On Sun, Jul 10, 2022 at 9:55 PM Ralph Goers <ra...@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ı <vo...@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.karwasz@gmail.com
> >
> > wrote:
> >
> >> Hi Volkan,
> >>
> >> On Sat, 9 Jul 2022 at 12:05, Volkan Yazıcı <vo...@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
> >>
>
>

Re: Source location static inlining

Posted by Ralph Goers <ra...@dslextreme.com>.
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ı <vo...@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 <pi...@gmail.com>
> wrote:
> 
>> Hi Volkan,
>> 
>> On Sat, 9 Jul 2022 at 12:05, Volkan Yazıcı <vo...@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
>> 


Re: Source location static inlining

Posted by Volkan Yazıcı <vo...@yazi.ci>.
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 <pi...@gmail.com>
wrote:

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

Re: Source location static inlining

Posted by "Piotr P. Karwasz" <pi...@gmail.com>.
Hi Volkan,

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

Re: Source location static inlining

Posted by Volkan Yazıcı <vo...@yazi.ci>.
On Sat, Jul 9, 2022 at 8:16 PM Ralph Goers <ra...@dslextreme.com>
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. Nevertheless, I will continue doing so. (I also have my
reservations whether Lombok constitutes a good reference model for us where
we can borrow some ideas from or if it just happens to be the only tool
that we know of which performs some sort of bytecode weaving similar to
what we have in mind.)

To the best of my knowledge... You need to scan a class to spot Log4j
usages and perform necessary magic. There is no other way to figure out if
Log4j is used or not. (Once the scan results in negative, you don't need to
override the bytecode of that class, obviously.)


> 2. Your solution looks like it is still done at runtime.


Yes, but that is only for the demonstration purposes of the bytecode
generation. Using the generated bytecode, you can override the class at
either compile time or runtime. (See the answer from Piotr.)


> I would like to do it at compile
> time. My though is to use the annotation processor and generate a new
> class that
> captures all the stack trace elements.I would then replace every logging
> call with
> a LogBuilder call. So
>    logger.info <http://logger.info/>(“Hello {}”, user);
> would become
>
>  logger.atInfo().withLocation(Log4jLocations.locations.get(“ThisFQCN”)).log(“Hello
> {}”, user);
>
> So, like Lombok, any class wanting this would be annotated with @Log4j2,
> but
> in addition to getting a logger declared they would also have the
> locations generated.
>

For one, why the extra annotation indirection when we can perform this
processing for existing class files without recompilation or whatsoever?

Second, spotting all call-sites, registering them to a
`Log4jLocations.locations` map could also be done. (Actually, this was my
first idea too. Though I continued with thread-locals since it is simpler.)
Though I have my doubts if transforming logger calls to their fluent-API
counterparts could be done _easily_. It feels to me this would need
specialization on every single logger API call.


>
> The issues I have are a) I haven’t figured out how Logbok performs its
> magic without
> generating new code


Lombok generates bytecode and overrides the original class file. That is
why you cannot make a Lombok-using project work in IDEA without installing
its plugin.


> or affecting line numbers,


Preserving line numbers is indeed a black magic. ASM, the library I used
for bytecode weaving, does this for us.


> b) I haven’t figured out how to get
> the location information while in the annotation processor.


ASM helps here. I have used ASM for two main purposes: 1) collecting the
source location information and 2) adding new bytecode before every logger
call.

>

Re: Source location static inlining

Posted by Ralph Goers <ra...@dslextreme.com>.

> On Jul 9, 2022, at 11:16 AM, Ralph Goers <ra...@dslextreme.com> 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 solution looks like it is still done at runtime. I would like to do it at compile 
> time. My though is to use the annotation processor and generate a new class that 
> captures all the stack trace elements.I would then replace every logging call with 
> a LogBuilder call. So
>    logger.info <http://logger.info/>(“Hello {}”, user);
> would become
>    logger.atInfo().withLocation(Log4jLocations.locations.get(“ThisFQCN”)).log(“Hello {}”, user);

Obviously this isn’t quite right. The key would need to be something like “a.b.c.ClassName-1” so each Location has its own key.

> 
> So, like Lombok, any class wanting this would be annotated with @Log4j2, but 
> in addition to getting a logger declared they would also have the locations generated.
> 
> The issues I have are a) I haven’t figured out how Logbok performs its magic without 
> generating new code or affecting line numbers, b) I haven’t figured out how to get 
> the location information while in the annotation processor. However, to solve b I think 
> it is just a matter of walking through the source code, which has to be done anyway 
> to find all the logger calls.
> 
> Ralph
> 
>> On Jul 9, 2022, at 3:04 AM, Volkan Yazıcı <volkan@yazi.ci <ma...@yazi.ci>> wrote:
>> 
>> Inspired by this SO post <https://stackoverflow.com/a/72437386/1278899 <https://stackoverflow.com/a/72437386/1278899>> and
>> with some help from Piotr <https://stackoverflow.com/a/72916795/1278899 <https://stackoverflow.com/a/72916795/1278899>>, 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
>> to replace the current source location capture that uses an exception-based
>> expensive mechanism with inlining the source location using bytecode
>> weaving. The sources are publicly available on GitHub.
>> <https://github.com/vy/asm-playground/tree/master/src/main/java/com/vlkan <https://github.com/vy/asm-playground/tree/master/src/main/java/com/vlkan>>
>> In a nutshell, the magic is as follows:
>> 
>> I have a logger library (Log4j.java
>> <https://github.com/vy/asm-playground/blob/master/src/main/java/com/vlkan/Log4j.java <https://github.com/vy/asm-playground/blob/master/src/main/java/com/vlkan/Log4j.java>>)
>> as follows:
>> 
>>    public static final ThreadLocal<SourceLocation> LOCATION_REF =
>> ThreadLocal.withInitial(SourceLocation::new);
>> 
>>    public static void log() {
>>        SourceLocation location = LOCATION_REF.get();
>>        boolean locationProvided = location.lineNumber > 0;
>>        if (!locationProvided) {
>>            StackTraceElement[] stackTraceElements = new
>> Throwable().getStackTrace();
>>            // Skip the first element pointing to this method.
>>            StackTraceElement stackTraceElement = stackTraceElements[1];
>>            location.init(
>>                    stackTraceElement.getFileName(),
>>                    stackTraceElement.getClassName(),
>>                    stackTraceElement.getMethodName(),
>>                    stackTraceElement.getLineNumber());
>>        }
>>        System.out.format(
>>                "[%s] %s%n",
>>                location,
>>                locationProvided ? "provided location" : "populated
>> location");
>>    }
>> 
>> Here note how `log()` uses a thread-local to see if there is already a
>> `SourceLocation` provided. If so, it leverages that, otherwise it populates
>> the source location using the stack trace of an exception.
>> 
>> Below is my actual application (AppActual.java
>> <https://github.com/vy/asm-playground/blob/master/src/main/java/com/vlkan/AppActual.java <https://github.com/vy/asm-playground/blob/master/src/main/java/com/vlkan/AppActual.java>>),
>> that is, what the actual/existing user code looks like:
>> 
>>    public static void main(String[] args) {
>>        System.out.println("should log at line 9");
>>        log();
>>        System.out.println("nothing to see here");
>>        System.out.println("should log at line 12");
>>        log();
>>        f();
>>    }
>> 
>>    private static void f() {
>>        System.out.println("adding some indirection");
>>        System.out.println("should log at line 19");
>>        log();
>>    }
>> 
>> I want to transform this into the following expected form (AppExpected.java
>> <https://github.com/vy/asm-playground/blob/master/src/main/java/com/vlkan/AppExpected.java <https://github.com/vy/asm-playground/blob/master/src/main/java/com/vlkan/AppExpected.java>>)
>> that exploits the `LOCATION_REF` thread-local to inline the source location
>> information:
>> 
>>    public static void main(String[] args) {
>>        System.out.println("should log at line 9");
>>        LOCATION_REF.get().init("AppExpected.java",
>> "com.vlkan.AppExpected", "main", 9);
>>        log();
>>        System.out.println("nothing to see here");
>>        System.out.println("should log at line 12");
>>        LOCATION_REF.get().init("AppExpected.java",
>> "com.vlkan.AppExpected", "main", 12);
>>        log();
>>        f();
>>    }
>> 
>>    private static void f() {
>>        System.out.println("adding some indirection");
>>        System.out.println("should log at line 19");
>>        LOCATION_REF.get().init("AppExpected.java",
>> "com.vlkan.AppExpected", "main", 19);
>>        log();
>>    }
>> 
>> And... 👉 AppTransforming.java
>> <https://github.com/vy/asm-playground/blob/master/src/main/java/com/vlkan/AppTransforming.java <https://github.com/vy/asm-playground/blob/master/src/main/java/com/vlkan/AppTransforming.java>>
>> 👈, my dear friends, performs exactly this transformation: converts
>> `AppActual` bytecode into `AppExpected`. 😍
>> 
>> 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.)
> 


Re: Source location static inlining

Posted by Ralph Goers <ra...@dslextreme.com>.
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 at compile 
time. My though is to use the annotation processor and generate a new class that 
captures all the stack trace elements.I would then replace every logging call with 
a LogBuilder call. So
   logger.info <http://logger.info/>(“Hello {}”, user);
would become
   logger.atInfo().withLocation(Log4jLocations.locations.get(“ThisFQCN”)).log(“Hello {}”, user);

So, like Lombok, any class wanting this would be annotated with @Log4j2, but 
in addition to getting a logger declared they would also have the locations generated.

The issues I have are a) I haven’t figured out how Logbok performs its magic without 
generating new code or affecting line numbers, b) I haven’t figured out how to get 
the location information while in the annotation processor. However, to solve b I think 
it is just a matter of walking through the source code, which has to be done anyway 
to find all the logger calls.

Ralph

> On Jul 9, 2022, at 3:04 AM, Volkan Yazıcı <vo...@yazi.ci> wrote:
> 
> Inspired by this SO post <https://stackoverflow.com/a/72437386/1278899> and
> with some help from Piotr <https://stackoverflow.com/a/72916795/1278899>, 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
> to replace the current source location capture that uses an exception-based
> expensive mechanism with inlining the source location using bytecode
> weaving. The sources are publicly available on GitHub.
> <https://github.com/vy/asm-playground/tree/master/src/main/java/com/vlkan>
> In a nutshell, the magic is as follows:
> 
> I have a logger library (Log4j.java
> <https://github.com/vy/asm-playground/blob/master/src/main/java/com/vlkan/Log4j.java>)
> as follows:
> 
>    public static final ThreadLocal<SourceLocation> LOCATION_REF =
> ThreadLocal.withInitial(SourceLocation::new);
> 
>    public static void log() {
>        SourceLocation location = LOCATION_REF.get();
>        boolean locationProvided = location.lineNumber > 0;
>        if (!locationProvided) {
>            StackTraceElement[] stackTraceElements = new
> Throwable().getStackTrace();
>            // Skip the first element pointing to this method.
>            StackTraceElement stackTraceElement = stackTraceElements[1];
>            location.init(
>                    stackTraceElement.getFileName(),
>                    stackTraceElement.getClassName(),
>                    stackTraceElement.getMethodName(),
>                    stackTraceElement.getLineNumber());
>        }
>        System.out.format(
>                "[%s] %s%n",
>                location,
>                locationProvided ? "provided location" : "populated
> location");
>    }
> 
> Here note how `log()` uses a thread-local to see if there is already a
> `SourceLocation` provided. If so, it leverages that, otherwise it populates
> the source location using the stack trace of an exception.
> 
> Below is my actual application (AppActual.java
> <https://github.com/vy/asm-playground/blob/master/src/main/java/com/vlkan/AppActual.java>),
> that is, what the actual/existing user code looks like:
> 
>    public static void main(String[] args) {
>        System.out.println("should log at line 9");
>        log();
>        System.out.println("nothing to see here");
>        System.out.println("should log at line 12");
>        log();
>        f();
>    }
> 
>    private static void f() {
>        System.out.println("adding some indirection");
>        System.out.println("should log at line 19");
>        log();
>    }
> 
> I want to transform this into the following expected form (AppExpected.java
> <https://github.com/vy/asm-playground/blob/master/src/main/java/com/vlkan/AppExpected.java>)
> that exploits the `LOCATION_REF` thread-local to inline the source location
> information:
> 
>    public static void main(String[] args) {
>        System.out.println("should log at line 9");
>        LOCATION_REF.get().init("AppExpected.java",
> "com.vlkan.AppExpected", "main", 9);
>        log();
>        System.out.println("nothing to see here");
>        System.out.println("should log at line 12");
>        LOCATION_REF.get().init("AppExpected.java",
> "com.vlkan.AppExpected", "main", 12);
>        log();
>        f();
>    }
> 
>    private static void f() {
>        System.out.println("adding some indirection");
>        System.out.println("should log at line 19");
>        LOCATION_REF.get().init("AppExpected.java",
> "com.vlkan.AppExpected", "main", 19);
>        log();
>    }
> 
> And... 👉 AppTransforming.java
> <https://github.com/vy/asm-playground/blob/master/src/main/java/com/vlkan/AppTransforming.java>
> 👈, my dear friends, performs exactly this transformation: converts
> `AppActual` bytecode into `AppExpected`. 😍
> 
> 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.)