Add runner benchmark

[noklam]

Description

Close #4127

Dev Notes:

• Rename benchmarks due to name clash with pyarrow: Unexpected benchmark module get installed with arrow apache/arrow#44398
• Add runner tests for peak memory, memory and time
• For time, each runner get a specific workload (IO/compute bound)
• benchmarks has been move to kedro_benchmarks, despite best effort to avoid installation of pyarrow, it will still causes a weird benchmarks module not found error. I try many different things, but ultimately I found that it works as long as the folder have any name other than `benchmarks. It's a bit strange but also not worth the effort to further investigate, so I end up renaming the folder.

QA notes

pip install asv or pip install -e ".[benchmark]"
asv run --quick

More notes

During test I discover ThreadRunner throws error like this occassionally, it's likely the same issue of #4191

DatasetAlreadyExistsError: Dataset 'dummy_1' has already been registered

Developer Certificate of Origin

We need all contributions to comply with the Developer Certificate of Origin (DCO). All commits must be signed off by including a Signed-off-by line in the commit message. See our wiki for guidance.

If your PR is blocked due to unsigned commits, then you must follow the instructions under "Rebase the branch" on the GitHub Checks page for your PR. This will retroactively add the sign-off to all unsigned commits and allow the DCO check to pass.

Checklist

• Read the contributing guidelines
• Signed off each commit with a Developer Certificate of Origin (DCO)
• Opened this PR as a 'Draft Pull Request' if it is work-in-progress
• Updated the documentation to reflect the code changes
• Added a description of this change in the RELEASE.md file
• Added tests to cover my changes
• Checked if this change will affect Kedro-Viz, and if so, communicated that with the Viz team

[noklam]

============================================================Done============================================================
{'SequentialRunner': 7.8415021896362305, 'ThreadRunner': 7.56311297416687, 'ParallelRunner': 3.4262261390686035}

At the moment I only running test for a compute-bound workload, the result is expected only ParallelRunner should speed things up. I run into issue sometimes with dataset already registered error. I think it's likely related to: #4191

@ElenaKhaustova any thought? Should I focus on testing only the KedroDataCatalog instead?

[noklam]

The reason to do this is multiprocessing breaks imports. Without this I get a benchmarks module is not found error.

To make sure this always in sys.path, I add this into part of the installation step. Other suggestions are welcomed.

I have tried PYTHONPATH as well but it doesn't seem to work as new process doesn't inherit it.

[merelcht]

This looks good 👍

One question I have is why the SequentialRunner and ParallelRunner are tested with the compute bound pipeline and ThreadRunner with io bound?

[ElenaKhaustova]

Shall we also test memory usage on various catalogs and actually return some objects that need sufficient memory rather than a "dummy" string?

[noklam]

Do you mean just changing what it returns? We can do that but I don't know if it changes anything. Do I also need to change the structure of the pipeline?

Most data are not stored in DataCatalog unless CacheDataset is used. From my understanding they only get loaded within the node function and get recycled as soon as node is finished.

[ElenaKhaustova]

I mean varying the catalog and number of entries there.

Makes sense about returns, thank you.

[noklam]

Do you mean adding also the KedroDataCatalog? I am not sure what do you want to test by varying the number of entries, can you elaborate more?

[ElenaKhaustova]

In general, I mean running tests for different catalog configurations—the number of datasets in the catalog, with/without patterns—so we vary something that actually affects memory usage. We can also vary the pipeline size and the number of input arguments for nodes.

In the current setup, we use the same pipeline and catalog configuration and we try to test memory usage by running nodes with heavy compute load that should not grow the memory. So it feels like we might add other scenarios that grow it and we're able to compare (memory grows as expected) runs on them.

Adding KedroDataCatalog would be nice as well, just to compare them. But we can do it in a separate PR if easier.

[noklam]

@ElenaKhaustova I see, https://github.com/kedro-org/kedro/blob/main/benchmarks/benchmark_kedrodatacatalog.py, I think this is covered in the catalog test already?

[ElenaKhaustova]

Yeah, but we didn't do memory profiling. Maybe worth adding it on the catalog side. Anyway, that's just an idea, so I don't want to block the PR with it.

[ElenaKhaustova]

This looks good 👍

One question I have is why the SequentialRunner and ParallelRunner are tested with the compute bound pipeline and ThreadRunner with io bound?

Maybe it makes sense to add time tests for each runner and each scenario (compute bound, io bound) so we can compare not only catalogs (when changes are applied) but also runners (in general, between each other).

[noklam]

This looks good 👍
One question I have is why the SequentialRunner and ParallelRunner are tested with the compute bound pipeline and ThreadRunner with io bound?

Maybe it makes sense to add time tests for each runner and each scenario (compute bound, io bound) so we can compare not only catalogs (when changes are applied) but also runners (in general, between each other).

We can do that. I didn't do it because I think we don't get much for running ThreadRunner for compute bound task (maybe things will change with 3.13 but we also need to enable that flag explicitly). These tests are more costly as they takes some time to run.

I can add these tests, and if these tests start taking too long then I will suggest to remove the less useful one.

[ElenaKhaustova]

Thank you, @noklam, LGTM!

There's a suggestion on possible extension of tests for memory profiling, but that can be done in the separate PR if decide to do that.