mem0 icon indicating copy to clipboard operation
mem0 copied to clipboard

⚡️ Speed up `_format_message()` by 81% in `embedchain/store/assistants.py`

Open misrasaurabh1 opened this issue 1 year ago • 4 comments

Description

📄 _format_message() in embedchain/store/assistants.py

📈 Performance went up by 81% (0.81x faster)

⏱️ Runtime went down from 47.00μs to 25.90μs

Explanation and details

(click to show)

The given code mostly consists of class and method definitions, so there's limited room for optimization because the computational complexity is relatively low. However, there are minor changes that can be applied for a bit of efficiency:

Using is not None comparisons in place of or checks in the constructor could prevent potential issues with zero values (0, '', False, etc.), which are False in a boolean context.

Used c.__class__ is MessageContentText instead of isinstance(c, MessageContentText). This checks for the exact type, not subclasses, which is faster if no subclassing is being used.

Type of change

Please delete options that are not relevant.

  • [x] Refactor (does not change functionality, e.g. code style improvements, linting)

How Has This Been Tested?

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

✅ 10 Passed − 🌀 Generated Regression Tests

(click to show generated tests)
# imports
import pytest  # used for our unit tests
from typing import cast
from unittest.mock import MagicMock

# Assuming the existence of these classes based on the function's usage
class ThreadMessage:
    def __init__(self, content):
        self.content = content

class MessageContentText:
    def __init__(self, value):
        self.text = MagicMock(value=value)

class MessageContentOther:
    pass
from embedchain.store.assistants import OpenAIAssistant
# unit tests

# Test normal case with text content
def test_format_message_with_text_content():
    content = [MessageContentText("Hello,"), MessageContentText("World!")]
    thread_message = ThreadMessage(content)
    assert OpenAIAssistant._format_message(thread_message) == "Hello, World!"

# Test empty content
def test_format_message_empty_content():
    thread_message = ThreadMessage([])
    assert OpenAIAssistant._format_message(thread_message) == ""

# Test content with non-text types
def test_format_message_with_non_text_content():
    content = [MessageContentText("Text"), MessageContentOther()]
    thread_message = ThreadMessage(content)
    assert OpenAIAssistant._format_message(thread_message) == "Text"

# Test content with None values
def test_format_message_with_none_values():
    content = [None, MessageContentText("Text"), None]
    thread_message = ThreadMessage(content)
    assert OpenAIAssistant._format_message(thread_message) == "Text"

# Test content with special characters and whitespace
def test_format_message_with_special_characters():
    content = [MessageContentText("Hello\n"), MessageContentText("\tWorld!")]
    thread_message = ThreadMessage(content)
    assert OpenAIAssistant._format_message(thread_message) == "Hello\n \tWorld!"

# Test content with long texts
def test_format_message_with_long_text():
    long_text = "A" * 10000
    content = [MessageContentText(long_text)]
    thread_message = ThreadMessage(content)
    assert OpenAIAssistant._format_message(thread_message) == long_text

# Test content with non-string text values
def test_format_message_with_non_string_text_values():
    content = [MessageContentText(123), MessageContentText(True)]
    thread_message = ThreadMessage(content)
    assert OpenAIAssistant._format_message(thread_message) == "123 True"

# Test invalid input types
def test_format_message_with_invalid_input():
    with pytest.raises(AttributeError):
        OpenAIAssistant._format_message("Not a ThreadMessage")

# Test no text content
def test_format_message_no_text_content():
    content = [MessageContentOther(), MessageContentOther()]
    thread_message = ThreadMessage(content)
    assert OpenAIAssistant._format_message(thread_message) == ""

# Test thread message with nested content
# This test assumes that nested content is not supported and should be ignored
def test_format_message_with_nested_content():
    nested_content = MessageContentText("Nested")
    content = [MessageContentText("Hello"), nested_content]
    thread_message = ThreadMessage(content)
    assert OpenAIAssistant._format_message(thread_message) == "Hello"

Checklist:

  • [x] My code follows the style guidelines of this project
  • [x] I have performed a self-review of my own code
  • [x] I have commented my code, particularly in hard-to-understand areas
  • [ ] I have made corresponding changes to the documentation
  • [x] My changes generate no new warnings
  • [x] I have added tests that prove my fix is effective or that my feature works
  • [x] New and existing unit tests pass locally with my changes
  • [x] Any dependent changes have been merged and published in downstream modules
  • [x] I have checked my code and corrected any misspellings

Maintainer Checklist

  • [ ] closes #xxxx (Replace xxxx with the GitHub issue number)
  • [ ] Made sure Checks passed

misrasaurabh1 avatar Feb 16 '24 09:02 misrasaurabh1

Codecov Report

Attention: Patch coverage is 0% with 1 line in your changes missing coverage. Please review.

Project coverage is 56.59%. Comparing base (8fd0e1f) to head (dd130d0). Report is 5 commits behind head on main.

:exclamation: Current head dd130d0 differs from pull request most recent head 3b32793

Please upload reports for the commit 3b32793 to get more accurate results.

Files Patch % Lines
embedchain/store/assistants.py 0.00% 1 Missing :warning:
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1261      +/-   ##
==========================================
+ Coverage   54.42%   56.59%   +2.16%     
==========================================
  Files         158      146      -12     
  Lines        6346     5955     -391     
==========================================
- Hits         3454     3370      -84     
+ Misses       2892     2585     -307     

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

codecov[bot] avatar Feb 16 '24 10:02 codecov[bot]

@misrasaurabh1 Can you please resolve merge conflicts so we can merge this PR?

Dev-Khant avatar Jun 10 '24 05:06 Dev-Khant

resolved the merge conflicts and linted the code

misrasaurabh1 avatar Jun 11 '24 22:06 misrasaurabh1

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
0 out of 2 committers have signed the CLA.

:x: codeflash-ai[bot]
:x: misrasaurabh1
You have signed the CLA already but the status is still pending? Let us recheck it.

CLAassistant avatar Jul 26 '24 02:07 CLAassistant

@misrasaurabh1 Please resolve the merge conflicts.

Dev-Khant avatar Aug 01 '24 20:08 Dev-Khant

Hey @misrasaurabh1 thanks for your contribution. Closing this PR for now as there is no publicly verifiable data about the claims made.

Dev-Khant avatar Aug 03 '24 05:08 Dev-Khant