-
Notifications
You must be signed in to change notification settings - Fork 25
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
Speed Up the Leaderboard #60
Conversation
- Changed the leaderboard to use heap-sort instead of dumping the data and then sorting - Batch queried all data used by ParseAndExecute, heavily speeding up the leaderboard
WalkthroughThe changes involve significant enhancements to data retrieval and query processing across the codebase. Key modifications include the introduction of batch retrieval methods for nodes and caches, optimizations of the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Command
participant Storage
participant Cache
User->>Command: Execute Run()
Command->>Storage: GetNodes()
Storage-->>Command: Return Nodes
Command->>Cache: GetCaches()
Cache-->>Command: Return Caches
Command->>Command: Process Queries
Command->>User: Return Results
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (8)
- cmd/leaderboard/custom/custom.go (3 hunks)
- cmd/query/query.go (2 hunks)
- pkg/graph/graph.go (3 hunks)
- pkg/graph/mockGraph.go (1 hunks)
- pkg/graph/parser.go (3 hunks)
- pkg/graph/parser_test.go (3 hunks)
- pkg/graph/storage.go (1 hunks)
- pkg/storages/redis_storage.go (3 hunks)
Additional context used
golangci-lint
cmd/query/query.go
37-37: ineffectual assignment to err
(ineffassign)
39-39: ineffectual assignment to err
(ineffassign)
Additional comments not posted (25)
pkg/graph/storage.go (1)
15-15
: LGTM!The addition of the
GetCaches
method to theStorage
interface is well-implemented and aligns with the existing methods. This will enhance the functionality by allowing batch retrieval of cache entries.The code changes are approved.
pkg/graph/parser_test.go (3)
42-44
: Verify the impact of reordering theCache
function call.The
Cache
function call is moved to a later point in the test sequence. Ensure that this reordering does not affect the state of thestorage
variable before the main execution logic is tested.Verify the test results to ensure that the reordering does not introduce any issues.
87-98
: LGTM!The changes to the
ParseAndExecute
function signature and the retrieval ofkeys
,nodes
, andcaches
from thestorage
object enhance the function's ability to handle more complex scenarios during execution.The code changes are approved.
99-99
: LGTM!The update to the
ParseAndExecute
function call ensures that the function has more context about the data it operates on, improving its robustness and error handling.The code changes are approved.
cmd/query/query.go (1)
41-49
: LGTM!The changes to retrieve caches associated with the keys, introduce new error handling for the cache retrieval process, and update the
ParseAndExecute
function call improve the method's capability to handle more complex scenarios by providing it with more context about the data it operates on, thereby improving its robustness and error handling.The code changes are approved.
cmd/leaderboard/custom/custom.go (8)
4-4
: LGTM!The import statement for
container/heap
is necessary for the heap operations.The code changes are approved.
47-50
: LGTM!Batch retrieval of nodes using
GetNodes
enhances performance by reducing the number of calls to the storage layer.The code changes are approved.
52-55
: LGTM!Batch retrieval of caches using
GetCaches
enhances performance by reducing the number of calls to the storage layer.The code changes are approved.
62-63
: LGTM!Initialization of
queryHeap
usingheap.Init
is necessary for managing heap operations.The code changes are approved.
70-70
: LGTM!The additional parameters in the
ParseAndExecute
call allow for more informed decision-making during query execution.The code changes are approved.
75-77
: LGTM!Using a heap structure for sorting queries based on the length of their outputs is efficient.
The code changes are approved.
80-83
: LGTM!Popping elements from the heap ensures that the queries are ordered correctly based on their output lengths.
The code changes are approved.
125-142
: LGTM!Encapsulating heap operations within the
queryHeap
type enhances code readability and maintainability.The code changes are approved.
pkg/graph/mockGraph.go (1)
145-159
: LGTM!The
GetCaches
method is correctly implemented and enhances the functionality ofMockStorage
by allowing retrieval of multiple caches based on their IDs.The code changes are approved.
pkg/graph/parser.go (6)
10-11
: LGTM!The additional parameters in the
ParseAndExecute
function signature allow for more complex operations involving node management and caching.The code changes are approved.
15-18
: LGTM!The
nameToIDs
mapping facilitates the retrieval of node IDs based on their names, streamlining the process of querying dependents and dependencies.The code changes are approved.
52-73
: LGTM!The handling of tokens prefixed with "dependents" or "dependencies" introduces a more structured approach to managing node relationships.
The code changes are approved.
76-83
: LGTM!Utilizing batch queries for dependents and dependencies reduces the number of individual queries made to the storage backend, improving performance.
The code changes are approved.
112-113
: LGTM!Filtering nodes based on their type ensures that only relevant nodes are included in the query results.
The code changes are approved.
100-100
: LGTM!Error handling and operator application are crucial for the correct execution of the script.
The code changes are approved.
Also applies to: 103-105
pkg/storages/redis_storage.go (3)
Line range hint
102-113
: LGTM!The function correctly retrieves and parses data from a list using
LRANGE
. The error handling is appropriate.The code changes are approved.
121-123
: LGTM!The function correctly adds data to a list using
RPUSH
. The error handling is appropriate.The code changes are approved.
202-233
: LGTM!The function efficiently retrieves multiple cache entries using a Redis pipeline. The error handling and data parsing are appropriate.
The code changes are approved.
pkg/graph/graph.go (2)
268-285
: LGTM!The function correctly handles batch queries for dependents, with appropriate error handling and result collection.
The code changes are approved.
305-322
: LGTM!The function correctly handles batch queries for dependencies, with appropriate error handling and result collection.
The code changes are approved.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- cmd/query/query.go (2 hunks)
Additional comments not posted (6)
cmd/query/query.go (6)
14-14
: Import statement forgithub.com/goccy/go-json
added.The import statement for
github.com/goccy/go-json
has been added. Ensure that this library is necessary and used appropriately in the code.Verify that the
github.com/goccy/go-json
library is necessary and used appropriately in the code.
37-40
: Retrieve all keys from storage and handle errors.The code retrieves all keys from the storage and handles errors appropriately. This is a good practice to ensure that the subsequent operations have the necessary data.
The code changes are approved.
42-45
: Retrieve nodes from storage and handle errors.The code retrieves nodes from the storage using the keys and handles errors appropriately. This ensures that the necessary data is available for the subsequent operations.
The code changes are approved.
47-50
: Retrieve caches from storage and handle errors with detailed messages.The code retrieves caches from the storage using the keys and handles errors with detailed messages. This improves the robustness and debuggability of the code.
The code changes are approved.
51-54
: Retrieve cache stack from storage and handle errors.The code retrieves the cache stack from the storage and handles errors appropriately. This ensures that the necessary data is available for the subsequent operations.
The code changes are approved.
55-57
: UpdateParseAndExecute
function call with additional context.The
ParseAndExecute
function call has been updated to include nodes, caches, and a boolean indicating whether the cache stack is empty. This enhances the method's capability to handle more complex scenarios by providing it with more context about the data it operates on.The code changes are approved.
Summary by CodeRabbit
New Features
MockStorage
andRedisStorage
, improving performance.ParseAndExecute
function for improved handling of complex node relationships.Bug Fixes
Refactor
Run
method to utilize a heap structure for sorting queries based on output lengths, improving performance.