Skip to content
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.

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

TokenEncoder #838

Merged
merged 1 commit into from
Apr 3, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 18 additions & 5 deletions pr_agent/algo/token_handler.py
Copy link
Contributor

Choose a reason for hiding this comment

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

Changes walkthrough

Enhancement
token_handler.py (+18/-5)
Implement Singleton Pattern for Token Encoding                     

pr_agent/algo/token_handler.py

  • Introduced TokenEncoder singleton class for thread-safe token
    encoding.
  • Implemented thread safety using a lock for creating a single instance
    of the encoder.
  • Updated TokenHandler to use TokenEncoder.get_token_encoder() method.
  • utils.py (+2/-2)
    Update Utils to Use TokenEncoder Singleton                             

    pr_agent/algo/utils.py

  • Replaced usage of get_token_encoder with
    TokenEncoder.get_token_encoder() for consistency.
  • Original file line number Diff line number Diff line change
    @@ -1,12 +1,25 @@
    from jinja2 import Environment, StrictUndefined
    from tiktoken import encoding_for_model, get_encoding

    from pr_agent.config_loader import get_settings
    from threading import Lock


    class TokenEncoder:
    _encoder_instance = None
    _model = None
    _lock = Lock() # Create a lock object

    Comment on lines +7 to 11
    Copy link
    Collaborator Author

    Choose a reason for hiding this comment

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

    /ask is there an alternatie for lock ?

    Copy link
    Contributor

    Choose a reason for hiding this comment

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

    Yes, there are alternatives to using a Lock for synchronization in Python, especially when dealing with shared resources across multiple threads. Here are a few alternatives:

    • RLock (Reentrant Lock): Similar to Lock, but allows a thread to acquire it multiple times. This is useful in situations where the same thread needs to re-acquire the lock without causing a deadlock.

    • Semaphore: A more advanced locking mechanism that allows a certain number of threads to access a resource at the same time. It's useful when you need to limit access to a resource but allow more than one thread to access it concurrently.

    • Event: An event is a simpler synchronization primitive that can be used to signal between threads. One thread signals an event, and other threads wait for it. It's not a lock but can be used to control access or the flow of execution between threads.

    • Condition: A condition variable allows one or more threads to wait until they are notified by another thread. It's more flexible than a simple lock as it allows threads to wait for some condition to become true.

    • Barrier: A barrier is used to make threads wait until a certain number of threads have reached a point of execution. This is useful in scenarios where you want to synchronize a specific part of the workflow across multiple threads.

    Each of these synchronization primitives has its own use cases and choosing the right one depends on the specific requirements of your application. For instance, if you need a simple mutual exclusion, a Lock might be sufficient. However, for more complex synchronization requirements, such as allowing multiple threads to access a resource but with a limit on the number of concurrent accesses, a Semaphore might be more appropriate.

    @classmethod
    def get_token_encoder(cls):
    model = get_settings().config.model
    if cls._encoder_instance is None or model != cls._model: # Check without acquiring the lock for performance
    with cls._lock: # Lock acquisition to ensure thread safety
    if cls._encoder_instance is None or model != cls._model:
    cls._model = model
    cls._encoder_instance = encoding_for_model(cls._model) if "gpt" in cls._model else get_encoding(
    "cl100k_base")
    return cls._encoder_instance

    def get_token_encoder():
    return encoding_for_model(get_settings().config.model) if "gpt" in get_settings().config.model else get_encoding(
    "cl100k_base")

    class TokenHandler:
    """
    Expand All @@ -31,7 +44,7 @@ def __init__(self, pr=None, vars: dict = {}, system="", user=""):
    - system: The system string.
    - user: The user string.
    """
    self.encoder = get_token_encoder()
    self.encoder = TokenEncoder.get_token_encoder()
    if pr is not None:
    self.prompt_tokens = self._get_system_user_tokens(pr, self.encoder, vars, system, user)

    Expand Down
    4 changes: 2 additions & 2 deletions pr_agent/algo/utils.py
    Original file line number Diff line number Diff line change
    Expand Up @@ -12,7 +12,7 @@
    from starlette_context import context

    from pr_agent.algo import MAX_TOKENS
    from pr_agent.algo.token_handler import get_token_encoder
    from pr_agent.algo.token_handler import TokenEncoder
    from pr_agent.config_loader import get_settings, global_settings
    from pr_agent.algo.types import FilePatchInfo
    from pr_agent.log import get_logger
    Expand Down Expand Up @@ -566,7 +566,7 @@ def clip_tokens(text: str, max_tokens: int, add_three_dots=True) -> str:
    return text

    try:
    encoder = get_token_encoder()
    encoder = TokenEncoder.get_token_encoder()
    num_input_tokens = len(encoder.encode(text))
    if num_input_tokens <= max_tokens:
    return text
    Expand Down
    Loading