Skip to content

Conversation

simonferquel
Copy link

Backport of dotnet#113924

@simonferquel
Copy link
Author

Closing and moving to other repo

@simonferquel simonferquel reopened this Apr 7, 2025
@simonferquel
Copy link
Author

Reopening as I dont have write access in other repo

{
profiler = new ClassLoad();
}
else if (clsid == DynamicJitOptimizations::GetClsid())

Choose a reason for hiding this comment

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

@simonferquel did you have the conflict in this file only?

Copy link
Author

Choose a reason for hiding this comment

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

It is for a test I introduced, change is legit

Choose a reason for hiding this comment

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

yes, the change is perfect!
I was curious about if this was the only conflicting place during backport as you've mentioned the conflicts

<ProjectReference Include="$(TestSourceDir)Common/CoreCLRTestLibrary/CoreCLRTestLibrary.csproj" />
<ProjectReference Include="../common/profiler_common.csproj" />
<CMakeProjectReference Include="$(MSBuildThisFileDirectory)/../native/CMakeLists.txt" />
<ProjectReference Include="../../tracing/eventpipe/common/eventpipe_common.csproj" />

Choose a reason for hiding this comment

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

do we need a dependency on eventpipe_common project?

Copy link
Author

Choose a reason for hiding this comment

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

Yes, I use it to intercept method loads in the test after requesting JITing of methods and check the optimization level.
There are similar tests in the CoreCLR test code base. Eventpipe_common brings helper to make intercepting events like that easier in tests.

Choose a reason for hiding this comment

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

ah, thanks, I can see the usage of IpcTraceTest

Copy link

@alexey-zakharov alexey-zakharov left a comment

Choose a reason for hiding this comment

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

great addition to the api! thanks for the change and tests!

@simonferquel
Copy link
Author

re-closing in favor of internal repo

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.

2 participants