Skip to content
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

Add Timer args to struct and add show method #57081

Merged
merged 1 commit into from
Jan 28, 2025

Conversation

IanButterworth
Copy link
Member

@IanButterworth IanButterworth commented Jan 17, 2025

Makes it possible to access & see the initial setup of a Timer.

This PR

julia> t = Timer(1, interval=2)
Timer (open, timeout: 1, interval: 2) @0x000000010c35be50

julia> wait(Timer(t -> println("$(t.timeout) seconds have passed"), 2))
2 seconds have passed

Master

julia> t = Timer(1, interval=2)
Timer(Ptr{Nothing}(0x0000600000b15800), Base.GenericCondition(Base.Threads.SpinLock(0)), true, false)

julia> dump(t)
Timer
  handle: Ptr{Nothing}(0x0000600000b15800)
  cond: Base.GenericCondition{Base.Threads.SpinLock}
    waitq: Base.IntrusiveLinkedList{Task}
      head: Nothing nothing
      tail: Nothing nothing
    lock: Base.Threads.SpinLock
      owned: Int64 0
  isopen: Bool true
  set: Bool true

@IanButterworth IanButterworth added needs tests Unit tests are required for this change needs docs Documentation for this change is required needs news A NEWS entry is required for this change labels Jan 17, 2025
@@ -90,6 +90,8 @@ mutable struct Timer
cond::ThreadSynchronizer
@atomic isopen::Bool
@atomic set::Bool
timeout::Real
Copy link
Member

Choose a reason for hiding this comment

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

I think we should store this as the computed UInt64 milliseconds. It can be printed in seconds with 3 decimals if you like. Not really worth introducing a type instability for this.

@@ -90,6 +90,8 @@ mutable struct Timer
cond::ThreadSynchronizer
@atomic isopen::Bool
@atomic set::Bool
timeout::Real
interval::Real
Copy link
Member

Choose a reason for hiding this comment

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

It turns out libuv has an accessor for this, uv_timer_get_repeat https://docs.libuv.org/en/v1.x/timer.html#c.uv_timer_get_repeat, so we can use that instead of storing it.

Copy link
Member Author

Choose a reason for hiding this comment

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

I tried this but it won't work if the timer has been closed

julia> t = Timer(1, interval = 0.1)
Timer (open, timeout: 1.0 s, interval: 0.1 s) @0x000000010cada9b0

julia> close(t)

julia> t

[91257] signal 11 (2): Segmentation fault: 11
in expression starting at REPL[4]:1
uv_timer_get_repeat at /workspace/srcdir/libuv/src/timer.c:120
getproperty at ./asyncevent.jl:126 [inlined]
show at ./asyncevent.jl:135
show at ./multimedia.jl:47

So I've stored both as their ms values.

@IanButterworth IanButterworth force-pushed the ib/timer_init_show branch 2 times, most recently from ea6f9d0 to 8dac283 Compare January 23, 2025 20:45
@IanButterworth

This comment was marked as resolved.

@IanButterworth IanButterworth removed needs docs Documentation for this change is required needs news A NEWS entry is required for this change needs tests Unit tests are required for this change labels Jan 23, 2025
@IanButterworth IanButterworth force-pushed the ib/timer_init_show branch 2 times, most recently from 6b9103e to cbe13dd Compare January 24, 2025 00:33
@IanButterworth IanButterworth merged commit 575d8e8 into JuliaLang:master Jan 28, 2025
7 checks passed
@IanButterworth IanButterworth deleted the ib/timer_init_show branch January 28, 2025 20:23
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