Re: [go-nuts] Re: slog's use of runtime.Callers() with a skip

2023-08-14 Thread 'Shaun Crampton' via golang-nuts
>
> Do you have any evidence to the contrary?


Only that when Go 1.12 dropped, our similar function stopped working and
that reducing the skip seemed to do the trick.

The symptom was that our function would see an assembly file as the caller,
which I interpreted to mean that we'd skipped too far.  It only happened in
goroutines with short stacks and I put it down to inlining.

Quite possible that the "fix" I made was mistaken, or that runtime.Callers
has been updated/fixed since then.

In case you want to stare at my fix, here is the diff:
https://github.com/projectcalico/libcalico-go/commit/1f1ababe294be198148c4232ef6c0344898d3b31#diff-7ab3329a79d456fe0d1747bba7344ac61e839f9097ae65d09b36f3c11ea13fbdL153


Looking back, I see three changes in that "fix":

   - Change to skip from 6 to 1 (and increase pcs buffer size accordingly).
   - Reslice pcs to the valid portion, looks like we missed that before;
   possible this was the "real" fix?
   - Change the list of file names that we skip.

We were already using CallersFrames before the fix.

The Go runtime does the right thing.
>

It does seem to in my local tests with up-to-date Go; i tried some toy
examples and checked that functions really were being inlined.

-- 
You received this message because you are subscribed to the Google Groups 
"golang-nuts" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to golang-nuts+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/golang-nuts/CAMhR0U1DtH_HLPFDO%2BRPT1xU0SxynoP_63uNE%2BzJ%2BES9wX%3D2Xg%40mail.gmail.com.


[go-nuts] Re: slog's use of runtime.Callers() with a skip

2023-08-11 Thread Jonathan Amsterdam
The Go runtime does the right thing.

On Friday, August 11, 2023 at 8:58:53 AM UTC-4 jake...@gmail.com wrote:

> As far as I can tell, skip works even in the face of inlined functions, at 
> least when used with runtime.CallersFrames(). It would be surprising  to me 
> if it did not. Do you have any evidence to the contrary?
>
> On Friday, August 4, 2023 at 9:51:34 AM UTC-4 sh...@tigera.io wrote:
>
>> I was looking at replacing logrus with the new slog library in our 
>> project.  I noticed that it uses runtime.Callers() with a fixed skip 
>>  
>> to collect the PC of the calling code, presumably to make it possible for 
>> the handler to emit line number and filename.
>>
>> Question is: is that sound in the face of inlining functions?  I think if 
>> the Info method gets inlined then the skip might be too large, for example.
>>
>> I remember having to change similar code in our project to use 
>> runtime.CallersFrames in order to deal with inlining. Quite possible 
>> there's a way to deal with an inlined PC that I wasn't aware of, but it 
>> seemed wrong to me.
>>
>> -Shaun
>>
>

-- 
You received this message because you are subscribed to the Google Groups 
"golang-nuts" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to golang-nuts+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/golang-nuts/2b25c687-1dd3-4c15-a0b8-cc13e3ca8e43n%40googlegroups.com.


[go-nuts] Re: slog's use of runtime.Callers() with a skip

2023-08-11 Thread jake...@gmail.com
As far as I can tell, skip works even in the face of inlined functions, at 
least when used with runtime.CallersFrames(). It would be surprising  to me 
if it did not. Do you have any evidence to the contrary?

On Friday, August 4, 2023 at 9:51:34 AM UTC-4 sh...@tigera.io wrote:

> I was looking at replacing logrus with the new slog library in our 
> project.  I noticed that it uses runtime.Callers() with a fixed skip 
>  
> to collect the PC of the calling code, presumably to make it possible for 
> the handler to emit line number and filename.
>
> Question is: is that sound in the face of inlining functions?  I think if 
> the Info method gets inlined then the skip might be too large, for example.
>
> I remember having to change similar code in our project to use 
> runtime.CallersFrames in order to deal with inlining. Quite possible 
> there's a way to deal with an inlined PC that I wasn't aware of, but it 
> seemed wrong to me.
>
> -Shaun
>

-- 
You received this message because you are subscribed to the Google Groups 
"golang-nuts" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to golang-nuts+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/golang-nuts/58b0c0ef-c8be-484f-8d90-5c7ca15dcf32n%40googlegroups.com.