Skip to content
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

Add basic output of where memory is stored after a compile. #4136

Merged
merged 6 commits into from
Jul 16, 2024

Conversation

jonmeow
Copy link
Contributor

@jonmeow jonmeow commented Jul 15, 2024

The output is really basic, I'm just adding this to help track how memory is allocated.

---
filename:        'check/testdata/expr_category/in_place_tuple_init.carbon'
source_:
  used_bytes:      8057
  reserved_bytes:  8057
tokens_.allocator_:
  used_bytes:      0
  reserved_bytes:  0
tokens_.token_infos_:
  used_bytes:      1040
  reserved_bytes:  2032

(eliding)

value_stores_.string_literals_.set_:
  used_bytes:      320
  reserved_bytes:  320
Total:
  used_bytes:      20609
  reserved_bytes:  29437
...

// auto CollectMemUsage(llvm::StringRef label, AddMemUsageFn add_mem_usage)
// const -> void;
//
// The label should be concatenated with any child labels using MemUsageLabel in
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"MemUsageLabel" seems like it should be a function or something from this file, but this is the only mention of this word. Do you mean MemUsage::ConcatLabel?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed. Sorry, this is leftover from iterating on structure.

toolchain/base/mem_usage.h Outdated Show resolved Hide resolved
toolchain/base/mem_usage.h Outdated Show resolved Hide resolved
toolchain/base/mem_usage.h Outdated Show resolved Hide resolved
Comment on lines 29 to 30
// The arguments for AddMemUsageFn are the label and byte size. It should be
// called once per tracked size.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Out of date. Should say something about calling mem_usage.Add instead.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed

Comment on lines +68 to +95
template <typename T>
auto Collect(llvm::StringRef label, const T& arg) -> void {
arg.CollectMemUsage(*this, label);
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the advantage of using this function instead of calling CollectMemUsage directly?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was trying to enforce a standard API, keeping arguments with similar structure.

I tried making this another Add overload, but I think BumpPtrAllocator gets a little hostile to overload resolution in a way I'm not able to solve.

i.e., as-is, parameters are in similar positions, with the tracked data last:

    mem_usage.Collect(MemUsage::ConcatLabel(label, "values_"), values_);
    mem_usage.Add(MemUsage::ConcatLabel(label, "canonical_blocks_"),
                  canonical_blocks_, KeyContext(this));

Otherwise, these are fundamentally different:

    values_.CollectMemUsage(mem_usage, MemUsage::ConcatLabel(label, "values_"));
    mem_usage.Add(MemUsage::ConcatLabel(label, "canonical_blocks_"),
                  canonical_blocks_, KeyContext(this));

toolchain/base/value_store.h Outdated Show resolved Hide resolved
// called once per tracked size.
class MemUsage {
public:
auto Add(std::string label, int64_t used, int64_t reserved) -> void {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Clarify the units of used and reserved is bytes?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed

toolchain/base/mem_usage.h Outdated Show resolved Hide resolved
Co-authored-by: josh11b <15258583+josh11b@users.noreply.github.com>
@jonmeow jonmeow enabled auto-merge July 16, 2024 22:29
@jonmeow jonmeow added this pull request to the merge queue Jul 16, 2024
Merged via the queue into carbon-language:trunk with commit f1190a4 Jul 16, 2024
7 checks passed
@jonmeow jonmeow deleted the mem-alloc branch July 16, 2024 23:21
brymer-meneses pushed a commit to brymer-meneses/carbon-lang that referenced this pull request Aug 15, 2024
…anguage#4136)

The output is really basic, I'm just adding this to help track how
memory is allocated.

```
---
filename:        'check/testdata/expr_category/in_place_tuple_init.carbon'
source_:
  used_bytes:      8057
  reserved_bytes:  8057
tokens_.allocator_:
  used_bytes:      0
  reserved_bytes:  0
tokens_.token_infos_:
  used_bytes:      1040
  reserved_bytes:  2032

(eliding)

value_stores_.string_literals_.set_:
  used_bytes:      320
  reserved_bytes:  320
Total:
  used_bytes:      20609
  reserved_bytes:  29437
...
```

---------

Co-authored-by: josh11b <15258583+josh11b@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants