[webkit-reviews] review granted: [Bug 232976] Log memory usage metadata when WebContent crosses critical or warning level memory thresholds : [Attachment 443894] Patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Ben Nham <nham at apple.com>'s request
for review:
Bug 232976: Log memory usage metadata when WebContent crosses critical or
warning level memory thresholds
https://bugs.webkit.org/show_bug.cgi?id=232976

Attachment 443894: Patch

https://bugs.webkit.org/attachment.cgi?id=443894&action=review




--- Comment #4 from Darin Adler <darin at apple.com> ---
Comment on attachment 443894
  --> https://bugs.webkit.org/attachment.cgi?id=443894
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=443894&action=review

> Source/WTF/wtf/cocoa/ResourceUsageCocoa.cpp:140
> +	       // Count all resident malloc pages as dirty.
> +	       size_t dirtyPages = info.pages_resident - info.pages_reusable;
> +	       tags[info.user_tag].dirty += dirtyPages +
info.pages_swapped_out;

Do we really need a local variable just to add pages_swapped_out?

> Source/WTF/wtf/cocoa/ResourceUsageCocoa.cpp:143
> +	       tags[info.user_tag].dirty += (info.pages_dirtied +
info.pages_swapped_out);

Not sure the parentheses add clarity here.

> Source/WebCore/page/MemoryRelease.h:34
> +enum class LogMemoryStatisticsReason {

: uint8_t?

> Source/WebKit/WebProcess/WebProcess.cpp:464
> +		   // Log stats in the next turn of the run loop so that it
runs after the low memory handler.

This comment is here, but not above. Does the same rationale apply above?