-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Add caching to bottlenecks in the message store #5605
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
Changes from 3 commits
5b0914e
b306530
1cf2cc8
e42fad0
d442f4b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -1,5 +1,6 @@ | ||||||
# Licensed under the GPL: https://www.gnu.org/licenses/old-licenses/gpl-2.0.html | ||||||
# For details: https://github.com/PyCQA/pylint/blob/main/LICENSE | ||||||
import functools | ||||||
from typing import Dict, List, NoReturn, Optional, Tuple | ||||||
|
||||||
from pylint.exceptions import InvalidMessageError, UnknownMessageError | ||||||
|
@@ -101,18 +102,25 @@ def _raise_duplicate_msgid(symbol: str, msgid: str, other_msgid: str) -> NoRetur | |||||
) | ||||||
raise InvalidMessageError(error_message) | ||||||
|
||||||
@functools.lru_cache() | ||||||
def get_active_msgids(self, msgid_or_symbol: str) -> List[str]: | ||||||
"""Return msgids but the input can be a symbol.""" | ||||||
# Only msgid can have a digit as second letter | ||||||
is_msgid: bool = msgid_or_symbol[1:].isdigit() | ||||||
msgid = None | ||||||
if is_msgid: | ||||||
"""Return msgids but the input can be a symbol. | ||||||
|
||||||
The cache has no limit as it will never go very high. It's a set number of | ||||||
msgid/symbol pair from our own code and from user's checkers. How many msgid can | ||||||
the user generate ? I guess it will never be more than 1000, and if each | ||||||
msgid/symbol pair has around 50 characters, it's still ~= 5 kb to cache. | ||||||
Pierre-Sassoulas marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
""" | ||||||
msgid: Optional[str] | ||||||
symbol: Optional[str] | ||||||
Pierre-Sassoulas marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
if msgid_or_symbol[1:].isdigit(): | ||||||
# Only msgid can have a digit as second letter | ||||||
msgid = msgid_or_symbol.upper() | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Don't know why I didn't think of this sooner, but this should fix it (or this change but then on L118. We don't need an extra line, we just need to add typing to the call that casts either to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Well I did that at first but then There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why did we remove the def get_active_msgids(self, msgid_or_symbol: str) -> List[str]:
"""Return msgids but the input can be a symbol."""
# Only msgid can have a digit as second letter
msgid = None
if msgid_or_symbol[1:].isdigit():
msgid = msgid_or_symbol.upper()
symbol = self.__msgid_to_symbol.get(msgid)
else:
msgid = self.__symbol_to_msgid.get(msgid_or_symbol)
symbol = msgid_or_symbol
if msgid is None or symbol is None or not msgid or not symbol:
error_msg = f"No such message id or symbol '{msgid_or_symbol}'."
raise UnknownMessageError(error_msg)
return self.__old_names.get(msgid, [msgid]) Gives not errors for me with There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I suppose the type hint is only for type checker when the assignment is really executed at run time and impact performance. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That's true! |
||||||
symbol = self.__msgid_to_symbol.get(msgid) | ||||||
else: | ||||||
msgid = self.__symbol_to_msgid.get(msgid_or_symbol) | ||||||
symbol = msgid_or_symbol | ||||||
if msgid is None or symbol is None or not msgid or not symbol: | ||||||
if not msgid or not symbol: | ||||||
error_msg = f"No such message id or symbol '{msgid_or_symbol}'." | ||||||
raise UnknownMessageError(error_msg) | ||||||
return self.__old_names.get(msgid, [msgid]) |
Uh oh!
There was an error while loading. Please reload this page.