ykjit / yk

yk packages
https://ykjit.github.io/yk/
Other
29 stars 7 forks source link

Rethink normal logging. #1323

Closed ltratt closed 2 months ago

ltratt commented 2 months ago

Previously we lumped a lot of logging under YKD_LOG_JITSTATE including JIT state transitions, location transitions, warnings, and genuine errors. That meant that you either saw everything or (most often) nothing.

This commit removes YKD_LOG_JITSTATE and adds YK_LOG=[<path|->:]<level>. It defaults to logging to stderr at level 1 which is "log errors". I think everything up to level 3 is useful. I mostly don't find level 4 useful, but I'm keeping it for backwards compatibility.

Most of the C tests just change "jitstate" to "yk-jit-event" with the exception of setlongjmp.newcg.c which has this diff:

-//     jitstate: stop-tracing
-//     jitstate: trace-compilation-aborted: longjmp encountered
+//     yk-jit-event: stop-tracing
+//     yk-warning: trace-compilation-aborted: longjmp encountered
vext01 commented 2 months ago

Is the long-term plan to default to level 0? I suppose once the JIT is a bit more stable, we shouldn't print internal retryable JIT problems to users?

vext01 commented 2 months ago

Are there any follow up PRs to come after this? We discussed printing the address of the location for state transitions, and printing from-state -> to-state for state transitions.

vext01 commented 2 months ago

In hindsight, I wish I'd proposed the ability to turn on/off individual classes of things, so for example if I wanted to print only state transitions I could do YKD_LOG=-:loc-state-trans or similar.

That ship has sailed I fear.

ltratt commented 2 months ago

In hindsight, I wish I'd proposed the ability to turn on/off individual classes of things, so for example if I wanted to print only state transitions I could do YKD_LOG=-:loc-state-trans or similar.

Another PR could do that. I am not sure it's very useful, though, so I personally wouldn't consider it high priority.

vext01 commented 2 months ago

We've agreed offline to roll with this.