Skip to content

Conversation

kazutakahirata
Copy link
Contributor

There are two ways to create in-memory instances of
IndexedAllocationInfo -- deserialization of the raw MemProf data and
that of the indexed MemProf data.

With:

commit 74799f4
Author: Kazu Hirata kazu@google.com
Date: Sat Mar 23 19:50:15 2024 -0700

we compute CallStackId for each call stack in IndexedAllocationInfo
while deserializing the raw MemProf data.

This patch does the same while deserilizing the indexed MemProf data.

As with the patch above, this patch does not add any use of
CallStackId yet.

There are two ways to create in-memory instances of
IndexedAllocationInfo -- deserialization of the raw MemProf data and
that of the indexed MemProf data.

With:

  commit 74799f4
  Author: Kazu Hirata <kazu@google.com>
  Date:   Sat Mar 23 19:50:15 2024 -0700

we compute CallStackId for each call stack in IndexedAllocationInfo
while deserializing the raw MemProf data.

This patch does the same while deserilizing the indexed MemProf data.

As with the patch above, this patch does not add any use of
CallStackId yet.
Copy link

✅ With the latest revision this PR passed the C/C++ code formatter.

Copy link

✅ With the latest revision this PR passed the Python code formatter.

endian::readNext<FrameId, llvm::endianness::little, unaligned>(Ptr);
Node.CallStack.push_back(Id);
}
Node.CSId = hashCallStack(Node.CallStack);
Copy link
Contributor

Choose a reason for hiding this comment

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

Will this eventually be replaced by the CSId held in the indexed profile?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes. As we transition from the current version to the next, we will most likely have two deserializers, say IndexedMemProfRecord::deserializeOld and IndexedMemProfRecord::deserializeNew. The new one will obtain CSId directly from the disk.

@kazutakahirata kazutakahirata merged commit 6646fe8 into llvm:main Mar 25, 2024
@kazutakahirata kazutakahirata deleted the pr_memprof_callstacks_indexed branch March 25, 2024 21:21
Copy link

@snehasish snehasish left a comment

Choose a reason for hiding this comment

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

lgtm

/*Payload=*/Start + FramePayloadOffset,
/*Base=*/Start, memprof::FrameLookupTrait()));

#if EXPENSIVE_CHECKS

Choose a reason for hiding this comment

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

nit: I think the convention is to use #ifdef which is slightly different in meaning to #if only.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants