Skip to content

first draft #161

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
Apr 19, 2025
Merged

first draft #161

merged 3 commits into from
Apr 19, 2025

Conversation

@alvin-r
Copy link
Contributor

@alvin-r alvin-r commented Apr 18, 2025 •
edited by github-actions bot
Loading

PR Type

Documentation

Description

  • Add comprehensive docs for Codeflash benchmark mode

  • Show pyproject.toml benchmark-root configuration

  • Provide pytest-benchmark test examples and flags

  • Outline internal benchmark workflow steps

Changes walkthrough 📝

Relevant files
Documentation
benchmarking.md
Document benchmark mode usage                                                       

docs/docs/optimizing-with-codeflash/benchmarking.md

  • Create new markdown guide for benchmark mode
  • Detail setup: benchmarks-root in pyproject.toml
  • Include pytest-benchmark test examples
  • Explain CLI usage and CI integration
  • Describe internal workflow steps
    		• +91/-0   
    Need help?
  • Type /help how to ... in the comments thread for any questions about PR-Agent usage.
  • Check out the documentation for more information.
    •

    @github-actions
    Copy link

    github-actions bot commented Apr 18, 2025 •
    edited
    Loading

    PR Reviewer Guide 🔍

    (Review updated until commit 3daf445)

    Here are some key observations to aid the review process:

    ⏱️ Estimated effort to review: 1 🔵⚪⚪⚪⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    Grammar

    The sentence "Codeflash is able the determine the impact ..." has a typo; it should read "able to determine the impact".

     
    Codeflash is able the determine the impact of an optimization on predefined benchmarks, when used in benchmark mode.
    Code block formatting

    The YAML snippet under "Create a benchmarks root" is indented inconsistently. Remove extra spaces so the ```yaml fence and its contents render properly.

     
     ```yaml
 [tool.codeflash]
# All paths are relative to this pyproject.toml's directory.
module-root = "inference"
tests-root = "tests"
test-framework = "pytest"
benchmarks-root = "tests/benchmarks" # add your benchmarks root dir here
ignore-paths = []
formatter-cmds = ["disabled"]
    
</details>

<details><summary><a href='https://github.com/codeflash-ai/codeflash/pull/161/files#diff-888e2e855be12d8b019fc4bbe6fe518443d4502864799cc0b607f8ad91ff71caR45-R47'><strong>Plugin clarification</strong></a>

The note about pytest‑benchmark plugin usage is confusing. Clarify that Codeflash uses its own pytest plugin rather than the external pytest‑benchmark plugin.
</summary>

```markdown

The pytest-benchmark format is simply used as an interface. The plugin is actually not used - Codeflash will run these benchmarks with its own pytest plugin

    @github-actions
    Copy link

    PR Code Suggestions ✨

    No code suggestions found for the PR.

    @alvin-r alvin-r marked this pull request as ready for review April 19, 2025 19:35
    @alvin-r alvin-r requested a review from misrasaurabh1 April 19, 2025 19:35
    @github-actions
    Copy link

    Persistent review updated to latest commit 3daf445

    @github-actions
    Copy link

    PR Code Suggestions ✨

    No code suggestions found for the PR.

    codeflash-ai bot added a commit that referenced this pull request Apr 19, 2025
    @codeflash-ai
    ⚡️ Speed up method `QualifiedFunctionUsageMarker._expand_qualified_fu…
    1c3ccd4 
    …nctions` by 53% in PR #161 (`benchmark-docs`)

To optimize this program, we'll focus on reducing the runtime of the `_expand_qualified_functions` method. Let's analyze the main performance issues based on the provided profiling results.

1. The loop `for name in self.definitions` is called 22,095,676 times, which is significantly higher than the outer loop iterations (3,396), suggesting inefficiency in handling `self.definitions`.

2. The `name.startswith(f"{class_name}.__") and name.endswith("__")` checks are done multiple times and each check is quite expensive within the high number of iterations.

### Optimizations.

1. Use more efficient data structures: 
   - Convert `self.definitions` to a preprocessed set or dictionary to quickly check for dunder methods.
   
2. Preprocess the definitions only once.
   - Instead of checking `name.startswith(f"{class_name}.__") and name.endswith("__")` inside the loop, preprocess the `self.definitions` to filter and classify dunder methods by class names.
   
### Optimized Code.



### Explanation.
1. We preprocess the definitions once in the `_preprocess_definitions` function to categorize dunder methods by their class names.
2. Reuse this preprocessed data in the `_expand_qualified_functions` function to check and expand dunder methods more efficiently.

This significantly reduces the complexity of the loops and the number of checks required during the expansion process.
    @codeflash-ai codeflash-ai bot mentioned this pull request Apr 19, 2025
    Closed
    @codeflash-ai
    Copy link
    Contributor

    codeflash-ai bot commented Apr 19, 2025

    ⚡️ Codeflash found optimizations for this PR

    📄 53% (0.53x) speedup for QualifiedFunctionUsageMarker._expand_qualified_functions in codeflash/context/unused_definition_remover.py

    ⏱️ Runtime : 1.98 microseconds → 1.29 microsecond (best of 96 runs)

    I created a new dependent PR with the suggested changes. Please review:

    If you approve, it will be merged into this PR (branch benchmark-docs).

    @alvin-r alvin-r merged commit 48967e6 into main Apr 19, 2025
    15 of 16 checks passed
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

    Reviewers

    @misrasaurabh1 misrasaurabh1 misrasaurabh1 approved these changes

    Assignees

    No one assigned

    Labels

    Review effort 1/5

    Projects

    None yet

    Milestone

    No milestone

    Development

    Successfully merging this pull request may close these issues.

    3 participants

    @alvin-r @misrasaurabh1