-
Notifications
You must be signed in to change notification settings - Fork 1
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
chore: add more logs for OOM search #164
Conversation
CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅ |
I have read the CLA Document and I hereby sign the CLA |
src/types/model.ts
Outdated
@@ -95,27 +96,32 @@ export interface RegistryBlock { | |||
hash: string; | |||
} | |||
|
|||
type OrdersPerOwner = Map<Owner, Set<ConditionalOrder>>; | |||
|
|||
const logger = getLogger("Registry"); |
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.
Why registry if this is in model.ts
?
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.
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.
Its a similar comment as above. You took it out of the registry and has still a very generic name.
I don't think getting loggers should be the memory leak (i hope)
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.
Got you.
Created a separate logger:
const loadOwnerOrdersLogger = getLogger("loadOwnerOrders");
This code doesn't change the logic, it only adds more logs and refactors the code a bit.
The last point is my suspect. I assume that comparation here is not valid, we should not use shallow comparison for the objects.