Skip to content
This repository has been archived by the owner on Jan 23, 2023. It is now read-only.

Add Medium GC Profiling & ICorProfilerInfo::GetObjectReferences #24156

Merged
merged 1 commit into from
Apr 25, 2019

Conversation

mjsabby
Copy link

@mjsabby mjsabby commented Apr 21, 2019

@davmason davmason added this to the 3.0 milestone Apr 22, 2019
Copy link
Member

@davmason davmason left a comment

Choose a reason for hiding this comment

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

Minus the one comment it looks fine to me

src/vm/proftoeeinterfaceimpl.cpp Show resolved Hide resolved
@mjsabby mjsabby marked this pull request as ready for review April 22, 2019 20:37
@mjsabby mjsabby force-pushed the profilerGetObjectReferences branch from 6f7d712 to 768ceb1 Compare April 23, 2019 03:44
src/gc/gc.cpp Show resolved Hide resolved
@mjsabby mjsabby force-pushed the profilerGetObjectReferences branch 2 times, most recently from d2d1ff2 to 09a37fe Compare April 23, 2019 05:44
@mjsabby mjsabby changed the title Add ICorProfilerInfo::GetObjectReferences Add Medium GC Profiling & ICorProfilerInfo::GetObjectReferences Apr 23, 2019
@mjsabby mjsabby force-pushed the profilerGetObjectReferences branch from 09a37fe to 702e568 Compare April 23, 2019 06:01
Copy link
Member

@noahfalk noahfalk left a comment

Choose a reason for hiding this comment

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

Looked reasonable to me. We'll also need a corresponding PR for updated docs.

src/gc/gc.cpp Show resolved Hide resolved
src/inc/corprof.idl Outdated Show resolved Hide resolved
src/vm/gcenv.ee.cpp Show resolved Hide resolved
@mjsabby mjsabby force-pushed the profilerGetObjectReferences branch 2 times, most recently from c712a50 to 1b50761 Compare April 23, 2019 17:27
Copy link
Member

@Maoni0 Maoni0 left a comment

Choose a reason for hiding this comment

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

LGTM

@mjsabby mjsabby force-pushed the profilerGetObjectReferences branch from 1b50761 to 77d39a3 Compare April 24, 2019 05:41
@davmason
Copy link
Member

LGTM for the new changes

@mjsabby
Copy link
Author

mjsabby commented Apr 24, 2019

Can someone check this in please?

@Maoni0
Copy link
Member

Maoni0 commented Apr 24, 2019

I'm happy to merge this but I wanted to make sure that diagnostics folks are ok with the API stuff here. @noahfalk do you need more time to think about this? I think it's ok to have it as 2 flags - light and medium, and they are exclusive (ie, medium does not imply light). if they are ok with this then we'll merge.

Copy link
Member

@noahfalk noahfalk left a comment

Choose a reason for hiding this comment

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

I had one more question inline, but seems fine to me. 👍

src/inc/corprof.idl Show resolved Hide resolved
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add Profiler API : GetObjectReferences
4 participants