Safe-DS / Runner

Execute Safe-DS programs that were compiled to Python.
MIT License
2 stars 0 forks source link

feat: memoization improvements #81

Closed WinPlay02 closed 6 months ago

WinPlay02 commented 6 months ago

Summary of Changes

Closes #44

Depends on https://github.com/Safe-DS/Library/pull/609

github-actions[bot] commented 6 months ago

🦙 MegaLinter status: ✅ SUCCESS

Descriptor Linter Files Fixed Errors Elapsed time
✅ PYTHON black 7 0 0 0.69s
✅ PYTHON mypy 7 0 2.57s
✅ PYTHON ruff 7 0 0 0.03s
✅ REPOSITORY git_diff yes no 0.01s

See detailed report in MegaLinter reports _Set VALIDATE_ALL_CODEBASE: true in mega-linter.yml to validate all sources, not only the diff_

_MegaLinter is graciously provided by OX Security_

codecov[bot] commented 6 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 100.00%. Comparing base (6f820bf) to head (c7f8cb9).

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #81 +/- ## ========================================== Coverage 100.00% 100.00% ========================================== Files 11 14 +3 Lines 530 721 +191 ========================================== + Hits 530 721 +191 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

lars-reimann commented 6 months ago

@WinPlay02 Is this ready for review or are you still working on it?

lars-reimann commented 6 months ago

Some quick testing shows some major performance improvements compared to main.

After clicking on "Explore Table", there's always still roughly a 3s delay between the program message and the first placeholder_type, but afterward the table view and profiling appear much faster.

That initial delay might be related to the startup delay we observed for the library, since each worker process has to load the library?

WinPlay02 commented 6 months ago

Some quick testing shows some major performance improvements compared to main.

After clicking on "Explore Table", there's always still roughly a 3s delay between the program message and the first placeholder_type, but afterward the table view and profiling appear much faster.

That initial delay might be related to the startup delay we observed for the library, since each worker process has to load the library?

Yes, that should be the Startup delay. That's now the hardest problem to solve.

WinPlay02 commented 6 months ago

@WinPlay02 Is this ready for review or are you still working on it?

It was still in draft, as the strategies now use functions instead of lambdas (passing to the pipeline process lead to a PickleError), which arguably is not as clean, but I don't know a better solution for now.

I also tried to use a process pool, as discussed on Thursday, but I didn't find a satisfying solution on how it should be handled, if the max. amount of processes has been reached. (Waiting in a queue, vs sending an Error)

Using any pool (library) doesn't map to this pipeline based usage model very well, as they are made with parallelism or concurrency in mind.

Apart from these two things, it should be reviewable

lars-reimann commented 6 months ago

:tada: This PR is included in version 0.11.0 :tada:

The release is available on:

Your semantic-release bot :package::rocket: