rust-transit / gtfs-structure

Read a GTFS file
MIT License
56 stars 32 forks source link

Use a better time/duration representation #122

Open antoine-de opened 2 years ago

antoine-de commented 2 years ago

Maybe we should use std::time::Duration instead of u32 for time representation.

I'm not sure about stop_time.arrival_time / stop_time.departure_time (since it's a duration from midnight), but I least for transfers and pathways I think it would lead to better ergonomics.

kylerchin commented 9 months ago

A good question to ask is, what impact would this have on memory and CPU performance of doing routing calculations? Do users want them in Duration instead of number of seconds?

worth looking into!

lolpro11 commented 7 months ago

A good question to ask is, what impact would this have on memory and CPU performance of doing routing calculations? Do users want them in Duration instead of number of seconds?

worth looking into!

your_function:
        sub     rsp, 40
        mov     dword ptr [rsp + 20], esi
        mov     qword ptr [rsp + 24], rdi
        mov     qword ptr [rsp + 32], rdi
        add     esi, esi
        setb    al
        test    al, 1
        jne     .LBB6_2
        mov     edi, dword ptr [rsp + 20]
        call    qword ptr [rip + <T as core::convert::Into<U>>::into@GOTPCREL]
        mov     rdi, rax
        call    core::time::Duration::from_secs
        mov     qword ptr [rsp + 8], rax
        mov     ecx, edx
        mov     rdx, qword ptr [rsp + 8]
        mov     dword ptr [rsp + 16], ecx
        mov     rdi, rdx
        mov     esi, ecx
        call    <core::time::Duration as core::ops::arith::Add>::add
        mov     edi, dword ptr [rsp + 20]
        mov     rsi, qword ptr [rsp + 8]
        mov     rcx, qword ptr [rsp + 24]
        mov     rax, qword ptr [rsp + 32]
        mov     edx, dword ptr [rsp + 16]
        mov     dword ptr [rcx], edi
        mov     qword ptr [rcx + 8], rsi
        mov     dword ptr [rcx + 16], edx
        add     rsp, 40
        ret
.LBB6_2:
        lea     rdi, [rip + str.0]
        lea     rdx, [rip + .L__unnamed_8]
        mov     rax, qword ptr [rip + core::panicking::panic@GOTPCREL]
        mov     esi, 28
        call    rax
fn your_function(num: u32) -> (u32, Duration) {
    let value = num;
    let x = value + value;
    let duration = Duration::from_secs(num.into());
    let y = duration + duration;
    (value, duration)
}

Not a fan of the function call everytime a arithmetic operation is called. Overall, would take more CPU cycles and require the Struct in cache.

Muzer commented 4 months ago

Why not a NaiveTime alongside an integer for number of days?

lolpro11 commented 4 months ago

Good Idea. Maybe this can be a crate feature that you can enable?Message ID: @.***>