-
Notifications
You must be signed in to change notification settings - Fork 425
Collect fine-grained statistics on memory usage #1782
Conversation
0c86da6
to
2e25f32
Compare
src/serializer/Referentializer.js
Outdated
realm: Realm; | ||
|
||
_newCapturedScopeInstanceIdx: number; | ||
referentializationState: Map<ReferentializationScope, ReferentializationState>; | ||
_referentializedNameGenerator: NameGenerator; | ||
|
||
getStatistics(): SerializerStatistics { | ||
invariant(this.realm.statistics instanceof SerializerStatistics); |
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.
These invariants make me feel uneasy. As always, I strongly recommend adding a comment explaining why this must always be true. I can't say that I understand the changes in this PR well enough to convince myself about this.
src/serializer/Referentializer.js
Outdated
@@ -60,7 +60,7 @@ export class Referentializer { | |||
_referentializedNameGenerator: NameGenerator; | |||
|
|||
getStatistics(): SerializerStatistics { |
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.
Even better would be to also change this method name to getSerializerStatistics.
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.
@NTillmann is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
Release notes: Collect fine-grained statistics on memory usage This refactoring does a few things: - Introduce a much nicer `measure` abstract to wrap computations for which we want to measure something - Measure not only time, but also heap usage - Project data to legacy statistics file format. - Limit scope of intermediate serialized information to further reduce memory usage
- Fixing reporting issues.
c580c5f
to
32563c9
Compare
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.
@NTillmann is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
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.
@NTillmann is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
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.
@NTillmann is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
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.
@NTillmann is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
Release notes: Collect fine-grained statistics on memory usage
This refactoring does a few things:
measure
abstract to wrap computations for which we want to measure something