Skip to content

⚡️ Speed up set_in_app_in_frames() by 97% in sentry_sdk/utils.py#10

Open
codeflash-ai[bot] wants to merge 1 commit intomasterfrom
codeflash/optimize-set_in_app_in_frames-2024-06-18T22.49.24
Open

⚡️ Speed up set_in_app_in_frames() by 97% in sentry_sdk/utils.py#10
codeflash-ai[bot] wants to merge 1 commit intomasterfrom
codeflash/optimize-set_in_app_in_frames-2024-06-18T22.49.24

Conversation

@codeflash-ai
Copy link
Copy Markdown

@codeflash-ai codeflash-ai Bot commented Jun 18, 2024

📄 set_in_app_in_frames() in sentry_sdk/utils.py

📈 Performance improved by 97% (0.97x faster)

⏱️ Runtime went down from 2.22 milliseconds to 1.13 millisecond

Explanation and details

Sure, here's an optimized version of the provided Python code.

Key Optimizations.

  • Avoid regular expressions for simple substring checks.
  • Eliminate redundant checks.
  • Use a set for the in_app_include and in_app_exclude lists for faster membership testing.

Explanation.

  1. _module_in_list: Combined the initial checks into one line to avoid multiple returns if the conditions are met.
  2. _is_external_source: Removed the use of regular expressions and replaced them with simple substring checks.
  3. _is_in_project_root: Simplified by making the project_root check inline.
  4. set_in_app_in_frames.
    • Converted in_app_include and in_app_exclude into sets for O(1) membership checks.
    • Simplified by removing redundant continue statements. If a frame's determination is made, it's either set immediately or checked in the next condition without unnecessary continue.

This approach should reduce the complexities and overhead associated with the initial implementation.

Correctness verification

The new optimized code was tested for correctness. The results are listed below.

✅ 63 Passed − ⚙️ Existing Unit Tests

(click to show existing tests)
- utils/test_general.py

✅ 17 Passed − 🌀 Generated Regression Tests

(click to show generated tests)
# imports
# function to test
import re

import pytest  # used for our unit tests


def _module_in_list(name, items):
    # type: (str, Optional[List[str]]) -> bool
    if name is None:
        return False

    if not items:
        return False

    for item in items:
        if item == name or name.startswith(item + "."):
            return True

    return False

def _is_external_source(abs_path):
    # type: (str) -> bool
    # check if frame is in 'site-packages' or 'dist-packages'
    external_source = (
        re.search(r"[\\/](?:dist|site)-packages[\\/]", abs_path) is not None
    )
    return external_source

def _is_in_project_root(abs_path, project_root):
    # type: (str, Optional[str]) -> bool
    if project_root is None:
        return False

    # check if path is in the project root
    if abs_path.startswith(project_root):
        return True

    return False
from sentry_sdk.utils import set_in_app_in_frames

# unit tests

def test_empty_frames():
    # Test with empty frames list
    assert set_in_app_in_frames([], [], []) is None

def test_single_frame_in_include():
    # Test single frame with module in in_app_include
    frames = [{"module": "my_module"}]
    in_app_include = ["my_module"]
    result = set_in_app_in_frames(frames, [], in_app_include)
    assert result[0]["in_app"] == True

def test_single_frame_in_exclude():
    # Test single frame with module in in_app_exclude
    frames = [{"module": "my_module"}]
    in_app_exclude = ["my_module"]
    result = set_in_app_in_frames(frames, in_app_exclude, [])
    assert result[0]["in_app"] == False

def test_frame_already_in_app_true():
    # Test frame already marked as in_app=True
    frames = [{"module": "my_module", "in_app": True}]
    result = set_in_app_in_frames(frames, [], [])
    assert result[0]["in_app"] == True

def test_frame_already_in_app_false():
    # Test frame already marked as in_app=False
    frames = [{"module": "my_module", "in_app": False}]
    result = set_in_app_in_frames(frames, [], [])
    assert result[0]["in_app"] == False

def test_frame_in_both_include_and_exclude():
    # Test frame in both in_app_include and in_app_exclude
    frames = [{"module": "my_module"}]
    in_app_include = ["my_module"]
    in_app_exclude = ["my_module"]
    result = set_in_app_in_frames(frames, in_app_exclude, in_app_include)
    assert result[0]["in_app"] == True  # Inclusion list takes precedence

def test_multiple_frames_mixed_include_exclude():
    # Test multiple frames with mixed inclusion and exclusion
    frames = [{"module": "include_module"}, {"module": "exclude_module"}]
    in_app_include = ["include_module"]
    in_app_exclude = ["exclude_module"]
    result = set_in_app_in_frames(frames, in_app_exclude, in_app_include)
    assert result[0]["in_app"] == True
    assert result[1]["in_app"] == False

def test_frame_with_abs_path_external_source():
    # Test frame with abs_path in external source
    frames = [{"abs_path": "/usr/local/lib/python3.9/site-packages/package/module.py"}]
    result = set_in_app_in_frames(frames, [], [])
    assert result[0]["in_app"] == False

def test_frame_with_abs_path_project_root():
    # Test frame with abs_path in project root
    frames = [{"abs_path": "/home/user/project/module.py"}]
    project_root = "/home/user/project"
    result = set_in_app_in_frames(frames, [], [], project_root)
    assert result[0]["in_app"] == True

def test_frame_with_module_in_include_and_abs_path_external():
    # Test frame with module in in_app_include and abs_path in external source
    frames = [{"module": "include_module", "abs_path": "/usr/local/lib/python3.9/site-packages/package/module.py"}]
    in_app_include = ["include_module"]
    result = set_in_app_in_frames(frames, [], in_app_include)
    assert result[0]["in_app"] == True  # Inclusion list takes precedence

def test_frame_with_module_in_exclude_and_abs_path_project_root():
    # Test frame with module in in_app_exclude and abs_path in project root
    frames = [{"module": "exclude_module", "abs_path": "/home/user/project/module.py"}]
    in_app_exclude = ["exclude_module"]
    project_root = "/home/user/project"
    result = set_in_app_in_frames(frames, in_app_exclude, [], project_root)
    assert result[0]["in_app"] == False  # Exclusion list takes precedence

def test_frame_with_none_values():
    # Test frame with None values
    frames = [{"module": None, "abs_path": None}]
    result = set_in_app_in_frames(frames, [], [])
    assert "in_app" not in result[0]

def test_frame_with_missing_keys():
    # Test frame with missing keys
    frames = [{}]
    result = set_in_app_in_frames(frames, [], [])
    assert "in_app" not in result[0]

def test_frame_with_empty_module():
    # Test frame with module as empty string
    frames = [{"module": "", "abs_path": "/home/user/project/module.py"}]
    result = set_in_app_in_frames(frames, [], [])
    assert "in_app" not in result[0]

def test_large_number_of_frames():
    # Test with a large number of frames
    frames = [{"module": f"module_{i}", "abs_path": f"/home/user/project/module_{i}.py"} for i in range(1000)]
    project_root = "/home/user/project"
    result = set_in_app_in_frames(frames, [], [], project_root)
    assert all(frame["in_app"] == True for frame in result)

def test_frames_with_mixed_valid_invalid_paths():
    # Test frames with mixed valid and invalid paths
    frames = [{"module": f"module_{i}", "abs_path": f"/home/user/project/module_{i}.py"} if i % 2 == 0 else {"module": f"module_{i}", "abs_path": f"/usr/local/lib/python3.9/site-packages/package/module_{i}.py"} for i in range(1000)]
    project_root = "/home/user/project"
    result = set_in_app_in_frames(frames, [], [], project_root)
    for i, frame in enumerate(result):
        if i % 2 == 0:
            assert frame["in_app"] == True
        else:
            assert frame["in_app"] == False

def test_frames_with_valid_paths_no_project_root():
    # Test frames with valid paths but no project root
    frames = [{"module": "module_1", "abs_path": "/home/user/project/module_1.py"}]
    result = set_in_app_in_frames(frames, [], [], None)
    assert "in_app" not in result[0]

def test_frames_with_nested_module_names():
    # Test frames with nested module names
    frames = [{"module": "package.subpackage.module"}]
    in_app_include = ["package.subpackage"]
    result = set_in_app_in_frames(frames, [], in_app_include)
    assert result[0]["in_app"] == True

🔘 (none found) − ⏪ Replay Tests

Sure, here's an optimized version of the provided Python code.

### Key Optimizations.
- Avoid regular expressions for simple substring checks.
- Eliminate redundant checks.
- Use a set for the in_app_include and in_app_exclude lists for faster membership testing.



### Explanation.
1. **_module_in_list**: Combined the initial checks into one line to avoid multiple returns if the conditions are met.
2. **_is_external_source**: Removed the use of regular expressions and replaced them with simple substring checks.
3. **_is_in_project_root**: Simplified by making the `project_root` check inline.
4. **set_in_app_in_frames**.
   - Converted `in_app_include` and `in_app_exclude` into sets for O(1) membership checks.
   - Simplified by removing redundant `continue` statements. If a frame's determination is made, it's either set immediately or checked in the next condition without unnecessary `continue`.

This approach should reduce the complexities and overhead associated with the initial implementation.
@codeflash-ai codeflash-ai Bot added the ⚡️ codeflash Optimization PR opened by Codeflash AI label Jun 18, 2024
@codeflash-ai codeflash-ai Bot requested a review from ihitamandal June 18, 2024 22:49
@ihitamandal
Copy link
Copy Markdown
Owner

Unclear whether usage of set improves timing by that much, but code overall looks good.

Copy link
Copy Markdown
Owner

@ihitamandal ihitamandal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verified speedup

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

⚡️ codeflash Optimization PR opened by Codeflash AI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant