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

Improve performance #230

Merged
merged 17 commits into from
Apr 20, 2024
Merged

Improve performance #230

merged 17 commits into from
Apr 20, 2024

Conversation

gwbres
Copy link
Collaborator

@gwbres gwbres commented Apr 13, 2024

This branch improves overal performances severely, especially in PPP opmode.
By improving the way we interface to the RTK core, we reduce the processing time severily.
PPP resolution of test_resources/ESBCDNK-2020 (24hr) is reduced from 2'30 to 3.5sec on my computer.

NB: this will remain open until we're 100% sure data precision is preserved.

gwbres added 7 commits April 13, 2024 10:32
   * reduce time to render this plot (slowest to render)
   by iterating the whole set only once, at the expense of more
   complex inner data types

Signed-off-by: Guillaume W. Bres <[email protected]>
Signed-off-by: Guillaume W. Bres <[email protected]>
Signed-off-by: Guillaume W. Bres <[email protected]>
Signed-off-by: Guillaume W. Bres <[email protected]>
Signed-off-by: Guillaume W. Bres <[email protected]>
@gwbres gwbres added enhancement New feature provided performance Performance enhancement labels Apr 13, 2024
@gwbres gwbres requested review from lnicola and larsnaesbye April 13, 2024 16:04
@gwbres gwbres self-assigned this Apr 13, 2024
@lnicola
Copy link
Member

lnicola commented Apr 13, 2024

For the record, I can't build this:

    Checking rstats v2.0.1
error: no rules expected the token `"point being added is coincident with gm"`
   --> /home/grayshade/.cargo/registry/src/index.crates.io-6f17d22bba15001f/rstats-2.0.1/src/vecg.rs:447:43
    |
447 |             return re_error("arith",here!("point being added is coincident with gm"))?;
    |                                           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ no rules expected this token in macro call
    |
    = note: while trying to match end of macro

error: no rules expected the token `"point being removed is coincident with gm"`
   --> /home/grayshade/.cargo/registry/src/index.crates.io-6f17d22bba15001f/rstats-2.0.1/src/vecg.rs:458:43
    |
458 |             return re_error("arith",here!("point being removed is coincident with gm"))?; 
    |                                           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ no rules expected this token in macro call
    |
    = note: while trying to match end of macro

error: no rules expected the token `"point being removed is coincident with gm"`
   --> /home/grayshade/.cargo/registry/src/index.crates.io-6f17d22bba15001f/rstats-2.0.1/src/vecg.rs:469:43
    |
469 |             return re_error("arith",here!("point being removed is coincident with gm"))?; 
    |                                           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ no rules expected this token in macro call
    |
    = note: while trying to match end of macro

error: no rules expected the token `"radius: invalid subscript"`
   --> /home/grayshade/.cargo/registry/src/index.crates.io-6f17d22bba15001f/rstats-2.0.1/src/vecvec.rs:264:47
    |
264 |             return re_error("DataError",here!("radius: invalid subscript"))?;
    |                                               ^^^^^^^^^^^^^^^^^^^^^^^^^^^ no rules expected this token in macro call
    |
    = note: while trying to match end of macro

error: no rules expected the token `"sigvec given no data"`
   --> /home/grayshade/.cargo/registry/src/index.crates.io-6f17d22bba15001f/rstats-2.0.1/src/vecvec.rs:303:43
    |
303 |             return re_error("empty",here!("sigvec given no data"))?;
    |                                           ^^^^^^^^^^^^^^^^^^^^^^ no rules expected this token in macro call
    |
    = note: while trying to match end of macro

@gwbres
Copy link
Collaborator Author

gwbres commented Apr 13, 2024

For the record, I can't build this:

    Checking rstats v2.0.1
error: no rules expected the token `"point being added is coincident with gm"`
   --> /home/grayshade/.cargo/registry/src/index.crates.io-6f17d22bba15001f/rstats-2.0.1/src/vecg.rs:447:43
    |
447 |             return re_error("arith",here!("point being added is coincident with gm"))?;
    |                                           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ no rules expected this token in macro call
    |
    = note: while trying to match end of macro

error: no rules expected the token `"point being removed is coincident with gm"`
   --> /home/grayshade/.cargo/registry/src/index.crates.io-6f17d22bba15001f/rstats-2.0.1/src/vecg.rs:458:43
    |
458 |             return re_error("arith",here!("point being removed is coincident with gm"))?; 
    |                                           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ no rules expected this token in macro call
    |
    = note: while trying to match end of macro

error: no rules expected the token `"point being removed is coincident with gm"`
   --> /home/grayshade/.cargo/registry/src/index.crates.io-6f17d22bba15001f/rstats-2.0.1/src/vecg.rs:469:43
    |
469 |             return re_error("arith",here!("point being removed is coincident with gm"))?; 
    |                                           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ no rules expected this token in macro call
    |
    = note: while trying to match end of macro

error: no rules expected the token `"radius: invalid subscript"`
   --> /home/grayshade/.cargo/registry/src/index.crates.io-6f17d22bba15001f/rstats-2.0.1/src/vecvec.rs:264:47
    |
264 |             return re_error("DataError",here!("radius: invalid subscript"))?;
    |                                               ^^^^^^^^^^^^^^^^^^^^^^^^^^^ no rules expected this token in macro call
    |
    = note: while trying to match end of macro

error: no rules expected the token `"sigvec given no data"`
   --> /home/grayshade/.cargo/registry/src/index.crates.io-6f17d22bba15001f/rstats-2.0.1/src/vecvec.rs:303:43
    |
303 |             return re_error("empty",here!("sigvec given no data"))?;
    |                                           ^^^^^^^^^^^^^^^^^^^^^^ no rules expected this token in macro call
    |
    = note: while trying to match end of macro

The Georust RINEX toolsuite does pass on my side and on the github worker.
But I do face the same problem when compiling the RTK core by itself, which is almost unbearable.
Therefore I only use the RTK core through the RINEX toolsuite, which is anyways my main application of it to this day.
This appeared about last december.

@ChristopherRabotin

rstat is a dependency that Nyx has to get rid of. Have no idea why it behaves so strange nor what it does

@ChristopherRabotin
Copy link
Contributor

ChristopherRabotin commented Apr 13, 2024 via email

@lnicola
Copy link
Member

lnicola commented Apr 13, 2024

Can you try to run "cargo update"? That usually solves the issue for me.

Thanks, that worked. I only tried cargo update -p rstats, but that here! macro is from.. indxvec, which is the scariest crate name I've ever seen 🙄.

@gwbres
Copy link
Collaborator Author

gwbres commented Apr 13, 2024

@ChristopherRabotin, @lnicola

thank you guys !

Signed-off-by: Guillaume W. Bres <[email protected]>
gwbres added 3 commits April 14, 2024 14:06
Signed-off-by: Guillaume W. Bres <[email protected]>
Signed-off-by: Guillaume W. Bres <[email protected]>
Signed-off-by: Guillaume W. Bres <[email protected]>
gwbres added 5 commits April 20, 2024 11:06
Signed-off-by: Guillaume W. Bres <[email protected]>
Signed-off-by: Guillaume W. Bres <[email protected]>
Signed-off-by: Guillaume W. Bres <[email protected]>
Signed-off-by: Guillaume W. Bres <[email protected]>
@gwbres gwbres merged commit adcd8c5 into main Apr 20, 2024
4 checks passed
@gwbres gwbres deleted the perf branch April 20, 2024 19:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature provided performance Performance enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants