Skip to content

⚡️ Speed up _get_url() by 7% in sentry_sdk/integrations/_asgi_common.py #2

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
Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

⚡️ Speed up _get_url() by 7% in sentry_sdk/integrations/_asgi_common.py #2

wants to merge 1 commit into from

Conversation

@codeflash-ai
Copy link

@codeflash-ai codeflash-ai bot commented Jun 14, 2024

📄 _get_url() in sentry_sdk/integrations/_asgi_common.py

📈 Performance improved by 7% (0.07x faster)

⏱️ Runtime went down from 68.5 microseconds to 63.9 microseconds

Explanation and details

Here's an optimized version of the given function. This code reduces the number of lookups and simplifies the structure while maintaining the same functionality.

Improvements Made.

  1. String Formatting: Used f-strings for string formatting instead of the % operator, resulting in faster execution.
  2. Redundant Lookup Removal: Combined the retrieval of "root_path" and "path" into a single expression, reducing dictionary lookups.
  3. Simplified Conditional Handling: Moved the server lookup after handling the host straight away to avoid unnecessary operations when host is provided.

Correctness verification

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

🔘 (none found) − ⚙️ Existing Unit Tests

✅ 27 Passed − 🌀 Generated Regression Tests

(click to show generated tests) 
# imports
import pytest  # used for our unit tests
from sentry_sdk.integrations._asgi_common import _get_url

# unit tests

# Basic Functionality
def test_default_scheme_no_host_or_server():
    assert _get_url({"path": "/example"}, "http", None) == "/example"
    assert _get_url({"root_path": "/root", "path": "/example"}, "http", None) == "/root/example"

def test_provided_host():
    assert _get_url({"path": "/example"}, "http", "example.com") == "http://example.com/example"
    assert _get_url({"root_path": "/root", "path": "/example"}, "http", "example.com") == "http://example.com/root/example"

# Scheme Handling
def test_scheme_present_in_asgi_scope():
    assert _get_url({"scheme": "https", "path": "/example"}, "http", None) == "/example"
    assert _get_url({"scheme": "wss", "path": "/example"}, "ws", None) == "/example"

def test_scheme_missing_in_asgi_scope():
    assert _get_url({"path": "/example"}, "https", None) == "/example"
    assert _get_url({"path": "/example"}, "wss", None) == "/example"

# Server Handling
def test_server_present_in_asgi_scope():
    assert _get_url({"server": ("example.com", 80), "path": "/example"}, "http", None) == "http://example.com/example"
    assert _get_url({"server": ("example.com", 443), "path": "/example"}, "https", None) == "https://example.com/example"
    assert _get_url({"server": ("example.com", 8080), "path": "/example"}, "http", None) == "http://example.com:8080/example"

def test_server_missing_in_asgi_scope():
    assert _get_url({"path": "/example"}, "http", None) == "/example"
    assert _get_url({"root_path": "/root", "path": "/example"}, "http", None) == "/root/example"

# Path Handling
def test_root_path_and_path_present():
    assert _get_url({"root_path": "/root", "path": "/example"}, "http", None) == "/root/example"
    assert _get_url({"root_path": "/root", "path": "/example"}, "http", "example.com") == "http://example.com/root/example"

def test_only_path_present():
    assert _get_url({"path": "/example"}, "http", None) == "/example"
    assert _get_url({"path": "/example"}, "http", "example.com") == "http://example.com/example"

def test_empty_paths():
    assert _get_url({"root_path": "", "path": ""}, "http", None) == ""
    assert _get_url({"root_path": "", "path": ""}, "http", "example.com") == "http://example.com"

# Edge Cases
def test_empty_asgi_scope():
    assert _get_url({}, "http", None) == ""
    assert _get_url({}, "http", "example.com") == "http://example.com"

def test_non_standard_ports():
    assert _get_url({"server": ("example.com", 8080), "path": "/example"}, "http", None) == "http://example.com:8080/example"
    assert _get_url({"server": ("example.com", 8443), "path": "/example"}, "https", None) == "https://example.com:8443/example"

def test_invalid_data_types():
    with pytest.raises(TypeError):
        _get_url({"server": "invalid"}, "http", None)
    with pytest.raises(TypeError):
        _get_url({"path": 123}, "http", None)

# Large Scale Test Cases
def test_long_paths():
    long_path = "/" + "a" * 1000
    assert _get_url({"path": long_path}, "http", "example.com") == f"http://example.com{long_path}"
    long_root_path = "/" + "a" * 500
    long_path = "/" + "b" * 500
    assert _get_url({"root_path": long_root_path, "path": long_path}, "http", "example.com") == f"http://example.com{long_root_path}{long_path}"

def test_large_asgi_scope():
    large_scope = {str(i): str(i) for i in range(1000)}
    large_scope["path"] = "/example"
    assert _get_url(large_scope, "http", "example.com") == "http://example.com/example"
    assert _get_url(large_scope, "http", None) == "/example"

# Performance and Scalability
def test_high_volume_of_requests():
    for i in range(1000):
        assert _get_url({"path": f"/example{i}"}, "http", "example.com") == f"http://example.com/example{i}"

🔘 (none found) − ⏪ Replay Tests

@codeflash-ai
⚡️ Speed up _get_url() by 7%
6bbe02f 
Here's an optimized version of the given function. This code reduces the number of lookups and simplifies the structure while maintaining the same functionality.


### Improvements Made.
1. **String Formatting**: Used f-strings for string formatting instead of the `%` operator, resulting in faster execution.
2. **Redundant Lookup Removal**: Combined the retrieval of "root_path" and "path" into a single expression, reducing dictionary lookups.
3. **Simplified Conditional Handling**: Moved the server lookup after handling the `host` straight away to avoid unnecessary operations when `host` is provided.
@codeflash-ai codeflash-ai bot added the ⚡️ codeflash Optimization PR opened by Codeflash AI label Jun 14, 2024
@codeflash-ai codeflash-ai bot requested a review from misrasaurabh1 June 14, 2024 01:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@misrasaurabh1 misrasaurabh1 Awaiting requested review from misrasaurabh1

Assignees

No one assigned

Labels

⚡️ codeflash Optimization PR opened by Codeflash AI

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant