Skip to content

Check whether debug logs would actually get logged. #2752

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 2 commits into from

Conversation

fps
Copy link
Contributor

@fps fps commented Apr 21, 2025

Pluto installs a logger that has min_level == Debug but then continues to not log debug messages except for when they come from within a pluto cell.

This patch checks whether the logger actually would log debug messages by calling shouldlog().

This is still WIP since

  1. I'm not sure this is the right approach
  2. shouldlog() expects an id argument and I'm not sure what to pass there. Right now I just pass 0

Copy link
Contributor

github-actions bot commented Apr 21, 2025

Your PR no longer requires formatting changes. Thank you for your contribution!

Copy link

codecov bot commented Apr 21, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 88.91%. Comparing base (689f985) to head (cd4c6d2).
Report is 4 commits behind head on master.

Additional details and impacted files
@@             Coverage Diff             @@
##           master    #2752       +/-   ##
===========================================
+ Coverage   74.13%   88.91%   +14.78%     
===========================================
  Files         153      153               
  Lines       13083    13175       +92     
===========================================
+ Hits         9699    11715     +2016     
+ Misses       3384     1460     -1924     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CUDA.jl Benchmarks

Benchmark suite Current: cd4c6d2 Previous: 689f985 Ratio
latency/precompile 47247462290.5 ns 47262491849.5 ns 1.00
latency/ttfp 6760920490 ns 6850988274 ns 0.99
latency/import 3203023533.5 ns 3234256358.5 ns 0.99
integration/volumerhs 9614138.5 ns 9610832.5 ns 1.00
integration/byval/slices=1 146790 ns 146633.5 ns 1.00
integration/byval/slices=3 425226 ns 425014 ns 1.00
integration/byval/reference 144971 ns 144902 ns 1.00
integration/byval/slices=2 286186 ns 285814 ns 1.00
integration/cudadevrt 103524 ns 103455 ns 1.00
kernel/indexing 14121.5 ns 14079 ns 1.00
kernel/indexing_checked 14752.5 ns 14684.5 ns 1.00
kernel/occupancy 696.8013698630137 ns 736.2635135135135 ns 0.95
kernel/launch 2211.3333333333335 ns 2200.777777777778 ns 1.00
kernel/rand 17923 ns 18289 ns 0.98
array/reverse/1d 19800 ns 19465 ns 1.02
array/reverse/2d 24682 ns 24852 ns 0.99
array/reverse/1d_inplace 11226 ns 10835.333333333334 ns 1.04
array/reverse/2d_inplace 13255 ns 11211 ns 1.18
array/copy 21213 ns 20658 ns 1.03
array/iteration/findall/int 159390.5 ns 158495.5 ns 1.01
array/iteration/findall/bool 140477 ns 138965 ns 1.01
array/iteration/findfirst/int 154469 ns 154022.5 ns 1.00
array/iteration/findfirst/bool 155316 ns 154886 ns 1.00
array/iteration/scalar 71788 ns 73635 ns 0.97
array/iteration/logical 210096 ns 214295 ns 0.98
array/iteration/findmin/1d 41570 ns 41632 ns 1.00
array/iteration/findmin/2d 94413 ns 93756 ns 1.01
array/reductions/reduce/1d 41031.5 ns 35894 ns 1.14
array/reductions/reduce/2d 44208.5 ns 40861.5 ns 1.08
array/reductions/mapreduce/1d 38359.5 ns 33779 ns 1.14
array/reductions/mapreduce/2d 46219.5 ns 48167 ns 0.96
array/broadcast 21059 ns 20678 ns 1.02
array/copyto!/gpu_to_gpu 13539 ns 11799 ns 1.15
array/copyto!/cpu_to_gpu 210983.5 ns 209537 ns 1.01
array/copyto!/gpu_to_cpu 243493.5 ns 245587 ns 0.99
array/accumulate/1d 109290 ns 109112 ns 1.00
array/accumulate/2d 80114 ns 79916 ns 1.00
array/construct 1286.75 ns 1334.4 ns 0.96
array/random/randn/Float32 43538 ns 43795.5 ns 0.99
array/random/randn!/Float32 26710.5 ns 26564 ns 1.01
array/random/rand!/Int64 27171 ns 26954 ns 1.01
array/random/rand!/Float32 8702.333333333334 ns 8711.666666666666 ns 1.00
array/random/rand/Int64 30162 ns 29929 ns 1.01
array/random/rand/Float32 13018 ns 12952 ns 1.01
array/permutedims/4d 61260 ns 61447 ns 1.00
array/permutedims/2d 55432 ns 55118.5 ns 1.01
array/permutedims/3d 56281 ns 56069 ns 1.00
array/sorting/1d 2757767 ns 2776265 ns 0.99
array/sorting/by 3367491 ns 3367373 ns 1.00
array/sorting/2d 1085617 ns 1085155 ns 1.00
cuda/synchronization/stream/auto 1042.6 ns 1076.7142857142858 ns 0.97
cuda/synchronization/stream/nonblocking 6402.8 ns 6585.2 ns 0.97
cuda/synchronization/stream/blocking 844.9113924050633 ns 792.0776699029126 ns 1.07
cuda/synchronization/context/auto 1153.4 ns 1197 ns 0.96
cuda/synchronization/context/nonblocking 6605.8 ns 6756.6 ns 0.98
cuda/synchronization/context/blocking 939.5454545454545 ns 963.3030303030303 ns 0.98

This comment was automatically generated by workflow using github-action-benchmark.

@fps fps mentioned this pull request Apr 23, 2025
@maleadt
Copy link
Member

maleadt commented Apr 23, 2025

I don't think this is (conceptually) correct, as it applies the shouldlog filter too early. IIUC, one could use shouldlog to filter based on the log id, which is derived from where @debug is used as opposed to where is_debug is called. I'm not entirely sure, though, as I'm not very familiar with logging internals.

That said, it still seems silly that Pluto enables debug logging only to not be interested in debug messages.

@fps
Copy link
Contributor Author

fps commented Apr 23, 2025

Yeah, I guess I'll take it to the Pluto side again. It is indeed silly to setup a debug logger and then ignore everything..

@fps fps closed this Apr 23, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants