Skip to content
This repository was archived by the owner on Nov 1, 2020. It is now read-only.

Conversation

@Suchiman
Copy link
Contributor

@Suchiman Suchiman commented Jul 7, 2019

This fixes 3/6 from #7339

@Suchiman
Copy link
Contributor Author

Suchiman commented Jul 7, 2019

@MichalStrehovsky

they probably won't fall apart. do it slowly :)

welp 😄

Copy link
Member

@jkotas jkotas left a comment

Choose a reason for hiding this comment

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

Thank you!


list(APPEND COMMON_RUNTIME_SOURCES
unix/PalRedhawkUnix.cpp
../gc/unix/gcenv.unix.cpp
Copy link
Member

Choose a reason for hiding this comment

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

I have noticed that gcenv.unix.cpp has several TODOs.

In particular, GCToOSInterface::GetCacheSizePerLogicalCpu is important for a good GC performance. It was not implemented for CoreRT before your change either, so it is not a regression. I would be nice to get it fixed in future change.

@Suchiman
Copy link
Contributor Author

Suchiman commented Jul 7, 2019

I was just looking at the gcenv.windows.cpp and it looks like the buildsystem doesn't define _X86_
image

@jkotas
Copy link
Member

jkotas commented Jul 7, 2019

It is fallback path for 32-bit x86 only that should not be needed at all, but Maoni was reluctant to delete it: dotnet/coreclr#22180 (comment)

@jkotas jkotas merged commit 4a576fe into dotnet:master Jul 8, 2019
@Suchiman Suchiman deleted the UseGCEnv branch July 8, 2019 06:41
jkotas added a commit to jkotas/coreclr that referenced this pull request Jul 12, 2019
jkotas added a commit to jkotas/coreclr that referenced this pull request Jul 12, 2019
jkotas added a commit to jkotas/coreclr that referenced this pull request Jul 12, 2019
jkotas added a commit to dotnet/coreclr that referenced this pull request Jul 17, 2019
* Rename GC's config.h to avoid name collisions

Port dotnet/corert#7596 to CoreCLR

* Fix build break
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants