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

Moved allocator and GCStatistics into their own files.#403

Closed
ala53 wants to merge 8 commits into
dotnet:masterfrom
ala53:master
Closed

Moved allocator and GCStatistics into their own files.#403
ala53 wants to merge 8 commits into
dotnet:masterfrom
ala53:master

Conversation

@ala53
Copy link
Copy Markdown

@ala53 ala53 commented Mar 6, 2015

It's a small change (mostly for code clarity) but now the allocator and statistics classes are no longer part of the 36,210 line monster that is gc.cpp - now gc.cpp is only 35,905 lines. Progress!

ala53 added 3 commits March 5, 2015 15:54
In gcpriv.h, there were 11 defines for dprintf that were commented out
and clearly not in use.
GC Statistics module has been moved to its own file.
allocator (the class) now resides in gcallocator.h & gcallocator.cpp

#includes were added to GCStatistics and allocator

Copyright notice was copy pasted.
@dnfclas
Copy link
Copy Markdown

dnfclas commented Mar 6, 2015

Hi @ala53, I'm your friendly neighborhood .NET Foundation Pull Request Bot (You can call me DNFBOT). Thanks for your contribution!

In order for us to evaluate and accept your PR, we ask that you sign a contribution license agreement. It's all electronic and will take just minutes. I promise there's no faxing. https://cla2.dotnetfoundation.org.

TTYL, DNFBOT;

@ala53
Copy link
Copy Markdown
Author

ala53 commented Mar 6, 2015

On the CLA, is there a way not to give my home address (I'd just prefer not to give out my personal information)?

For now, I just wrote "Prefer not to disclose."

@dnfclas
Copy link
Copy Markdown

dnfclas commented Mar 6, 2015

@ala53, Thanks for signing the contribution license agreement so quickly! Actual humans will now validate the agreement and then evaluate the PR.

Thanks, DNFBOT;

ala53 added 3 commits March 5, 2015 16:37
Forgot to include gcallocator.h
Added the header to implementation includes.
Moved alloc_list (class) to gcallocator.h
Moved #includes from .cpp to .h because GC.cpp defines max generation,
which is needed in the headers
@ala53
Copy link
Copy Markdown
Author

ala53 commented Mar 6, 2015

AAAAAAAAAAAaaaaaaaaaaaaaaaAAAAAAAAAAAAAAaaaaaaaaaaaaAAAAaaaaaaaaaaaaaaAAAA!!!!!1!1!!!!!1!!! The #includes will probably haunt me for the rest of my life. A literal copy paste change turned into a 2 hour project because of #include.

@jkotas
Copy link
Copy Markdown
Member

jkotas commented Mar 6, 2015

cc @Maoni0

ala53 added 2 commits March 5, 2015 17:59
Seemed to have been missing...
Turns out that max_generation was a hidden, private (anonymous) enum in
gc.h, causing a bunch of issues.
@brianrob
Copy link
Copy Markdown
Member

brianrob commented Mar 6, 2015

@ala53, thank you very much for your contribution.

It looks like this change is limited to formatting/re-organization changes. Per our contribution guidelines, we aren't planning to take pure formatting changes (https://github.com/dotnet/coreclr/wiki/Contribution-guidelines#formatting-changes). In addition to merge issues on the internal mirror, this also opens us up to copy-and-paste issues/bugs, as you experienced with the #includes.

If you would like to see gc.cpp re-organized, I would recommend opening an issue so that we can discuss how best to approach this.

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.

5 participants