[Impeller] track the sizes of all outstanding MTLTexture allocations and report per frame in MB, matching Vulkan implementation. #53618

jonahwilliams · 2024-06-27T20:46:13Z

Does what it says!

Fixes flutter/flutter#150936

jonahwilliams · 2024-06-27T20:46:24Z

FYI @gaaclarke

impeller/renderer/backend/metal/allocator_mtl.h

gaaclarke

Overall looks good. I just have a question about accounting for the other mip levels.

gaaclarke · 2024-06-28T17:02:54Z

impeller/renderer/backend/metal/allocator_mtl.h

+
+  ~DebugAllocatorStats() {}
+
+  void Increment(size_t size);


nit: note the units of size in a docstring.

gaaclarke · 2024-06-28T17:03:02Z

impeller/renderer/backend/metal/allocator_mtl.h

+
+  void Increment(size_t size);
+
+  void Decrement(size_t size);


gaaclarke · 2024-06-28T17:05:58Z

impeller/renderer/backend/metal/allocator_mtl.mm

+}
+
+void DebugAllocatorStats::Decrement(size_t size) {
+  size_.fetch_sub(size, std::memory_order_relaxed);


Do you think we should have an assert that size will not go below zero?

Well since its an unsigned type, it won't go below zero 🤓

Semantics! haha. It'll go below zero, but you won't like it.

gaaclarke · 2024-06-28T17:08:07Z

impeller/renderer/backend/metal/allocator_mtl.mm

+}
+
+size_t DebugAllocatorStats::GetAllocationSizeMB() {
+  return size_ * 1e-6;


nit: this is doing a conversion between 64bit floats and 64bit integers.

What change are you requesting?

friendly ping @gaaclarke

It's not a huge deal but divide by 1_000_000 instead.

gaaclarke · 2024-06-28T17:09:40Z

impeller/renderer/backend/metal/allocator_mtl.mm


+#ifdef IMPELLER_DEBUG
+  if (desc.storage_mode != StorageMode::kDeviceTransient) {
+    debug_allocater_->Increment(desc.GetByteSizeOfBaseMipLevel());


Does this mean we aren't tracking the size of the other mip levels? That will give us quite a different result?

impeller/renderer/backend/metal/allocator_mtl.h

…add_mem_bench

gaaclarke

lgtm, outstanding nit on an assert and double conversion

gaaclarke · 2024-06-28T23:21:23Z

impeller/renderer/backend/metal/allocator_mtl.mm

+}
+
+void DebugAllocatorStats::Decrement(size_t size) {
+  size_.fetch_sub(size, std::memory_order_relaxed);


Semantics! haha. It'll go below zero, but you won't like it.

gaaclarke · 2024-07-01T17:06:23Z

I commented on the commit but I don't know if that shows up for you anywhere. I made a mistake with 1_000_000. That's what it'd be for rust. For C++14 the delimiter is ', not '_'.

jonahwilliams · 2024-07-01T17:22:06Z

you fooled me , I was like ... I guess C++has this too!

…locations and report per frame in MB, matching Vulkan implementation. (flutter/engine#53618)

…151150) flutter/engine@3456fee...fc5bc14 2024-07-01 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Output .js files as ES6 modules. (#52023)" (flutter/engine#53674) 2024-07-01 [email protected] Roll Dart SDK from 21c08743ee4a to c23e58143793 (1 revision) (flutter/engine#53670) 2024-07-01 [email protected] Roll Skia from ae2b97d74812 to 8375bdc6e191 (3 revisions) (flutter/engine#53669) 2024-07-01 [email protected] Output .js files as ES6 modules. (flutter/engine#52023) 2024-07-01 [email protected] Roll Skia from a62bf018429c to ae2b97d74812 (3 revisions) (flutter/engine#53668) 2024-07-01 [email protected] [icu] Ignores the dir `flutter/third_party/icu/patches` (flutter/engine#53667) 2024-07-01 [email protected] [Impeller] track the sizes of all outstanding MTLTexture allocations and report per frame in MB, matching Vulkan implementation. (flutter/engine#53618) 2024-07-01 [email protected] Roll Dart SDK from 338c6d4fd9c5 to 21c08743ee4a (1 revision) (flutter/engine#53666) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll Please CC [email protected],[email protected],[email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md

…lutter#151150) flutter/engine@3456fee...fc5bc14 2024-07-01 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Output .js files as ES6 modules. (flutter#52023)" (flutter/engine#53674) 2024-07-01 [email protected] Roll Dart SDK from 21c08743ee4a to c23e58143793 (1 revision) (flutter/engine#53670) 2024-07-01 [email protected] Roll Skia from ae2b97d74812 to 8375bdc6e191 (3 revisions) (flutter/engine#53669) 2024-07-01 [email protected] Output .js files as ES6 modules. (flutter/engine#52023) 2024-07-01 [email protected] Roll Skia from a62bf018429c to ae2b97d74812 (3 revisions) (flutter/engine#53668) 2024-07-01 [email protected] [icu] Ignores the dir `flutter/third_party/icu/patches` (flutter/engine#53667) 2024-07-01 [email protected] [Impeller] track the sizes of all outstanding MTLTexture allocations and report per frame in MB, matching Vulkan implementation. (flutter/engine#53618) 2024-07-01 [email protected] Roll Dart SDK from 338c6d4fd9c5 to 21c08743ee4a (1 revision) (flutter/engine#53666) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll Please CC [email protected],[email protected],[email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md

Flutter driver side changes for flutter/engine#53618

…lutter#151150) flutter/engine@3456fee...fc5bc14 2024-07-01 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Output .js files as ES6 modules. (flutter#52023)" (flutter/engine#53674) 2024-07-01 [email protected] Roll Dart SDK from 21c08743ee4a to c23e58143793 (1 revision) (flutter/engine#53670) 2024-07-01 [email protected] Roll Skia from ae2b97d74812 to 8375bdc6e191 (3 revisions) (flutter/engine#53669) 2024-07-01 [email protected] Output .js files as ES6 modules. (flutter/engine#52023) 2024-07-01 [email protected] Roll Skia from a62bf018429c to ae2b97d74812 (3 revisions) (flutter/engine#53668) 2024-07-01 [email protected] [icu] Ignores the dir `flutter/third_party/icu/patches` (flutter/engine#53667) 2024-07-01 [email protected] [Impeller] track the sizes of all outstanding MTLTexture allocations and report per frame in MB, matching Vulkan implementation. (flutter/engine#53618) 2024-07-01 [email protected] Roll Dart SDK from 338c6d4fd9c5 to 21c08743ee4a (1 revision) (flutter/engine#53666) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll Please CC [email protected],[email protected],[email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md

…r#151153) Flutter driver side changes for flutter/engine#53618

track in destructor.

4a8579b

github-actions bot added the e: impeller label Jun 27, 2024

gaaclarke reviewed Jun 27, 2024

View reviewed changes

impeller/renderer/backend/metal/allocator_mtl.h Outdated Show resolved Hide resolved

jonahwilliams added 2 commits June 27, 2024 18:57

++

90c68ef

license

eb73420

jonahwilliams requested a review from gaaclarke June 28, 2024 16:28

Update allocator_mtl.mm

5bbf312

gaaclarke reviewed Jun 28, 2024

View reviewed changes

jonahwilliams added 2 commits June 28, 2024 10:47

++

0599098

Merge branch 'add_mem_bench' of github.com:jonahwilliams/engine into …

491390a

…add_mem_bench

gaaclarke approved these changes Jun 28, 2024

View reviewed changes

Jonah Williams added 5 commits June 29, 2024 09:35

Update allocator_mtl.mm

ebe3c03

Update allocator_mtl.mm

d3d1366

Update allocator_mtl.mm

1af8e93

Update allocator_mtl.mm

e3b4bcd

Update allocator_mtl.mm

51ed874

jonahwilliams added the autosubmit Merge PR when tree becomes green via auto submit App label Jul 1, 2024

auto-submit bot merged commit 0d36bd5 into flutter:main Jul 1, 2024

engine-flutter-autoroll mentioned this pull request Jul 1, 2024

Roll Flutter Engine from 3456fee1a6b9 to 0d36bd58fb51 (2 revisions) flutter/flutter#151130

Closed

engine-flutter-autoroll mentioned this pull request Jul 1, 2024

Roll Flutter Engine from 3456fee1a6b9 to ecef7d51c205 (4 revisions) flutter/flutter#151133

Closed

engine-flutter-autoroll mentioned this pull request Jul 1, 2024

Roll Flutter Engine from 3456fee1a6b9 to ac5d60ed2eaa (6 revisions) flutter/flutter#151139

Closed

engine-flutter-autoroll mentioned this pull request Jul 1, 2024

Roll Flutter Engine from 3456fee1a6b9 to 8aa03626e179 (7 revisions) flutter/flutter#151148

Closed

engine-flutter-autoroll mentioned this pull request Jul 2, 2024

Roll Flutter Engine from 3456fee1a6b9 to fc5bc14e6091 (8 revisions) flutter/flutter#151150

Merged

jonahwilliams mentioned this pull request Jul 2, 2024

[flutter_driver] add allocator mtl to memory event allowlist. flutter/flutter#151153

Merged

auto-submit bot pushed a commit to flutter/flutter that referenced this pull request Jul 2, 2024

[flutter_driver] add allocator mtl to memory event allowlist. (#151153)

61326af

Flutter driver side changes for flutter/engine#53618

victorsanni pushed a commit to victorsanni/flutter that referenced this pull request Jul 8, 2024

[flutter_driver] add allocator mtl to memory event allowlist. (flutte…

34c590f

…r#151153) Flutter driver side changes for flutter/engine#53618

[Impeller] track the sizes of all outstanding MTLTexture allocations and report per frame in MB, matching Vulkan implementation. #53618

[Impeller] track the sizes of all outstanding MTLTexture allocations and report per frame in MB, matching Vulkan implementation. #53618

Uh oh!

Conversation

jonahwilliams commented Jun 27, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jonahwilliams commented Jun 27, 2024

Uh oh!

Uh oh!

gaaclarke left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

gaaclarke left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gaaclarke commented Jul 1, 2024

Uh oh!

jonahwilliams commented Jul 1, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

jonahwilliams commented Jun 27, 2024 •

edited

Loading