Skip to content
This repository was archived by the owner on Oct 12, 2022. It is now read-only.
/ druntime Public archive

Comments

Add access to GC runtime profile stats#2332

Merged
dlang-bot merged 1 commit intodlang:masterfrom
PetarKirov:gc-runtime-profile-stats
Jan 4, 2019
Merged

Add access to GC runtime profile stats#2332
dlang-bot merged 1 commit intodlang:masterfrom
PetarKirov:gc-runtime-profile-stats

Conversation

@PetarKirov
Copy link
Member

@PetarKirov PetarKirov commented Oct 18, 2018

A rebase of #2155

@dlang-bot
Copy link
Contributor

dlang-bot commented Oct 18, 2018 •
edited
Loading

Thanks for your pull request, @ZombineDev!

Bugzilla references

Auto-close Bugzilla Severity Description
✓ 16377 enhancement Make --DRT GC profile information available outside of GC destruction

Testing this PR locally

If you don't have a local development environment setup, you can use Digger to test this PR:

dub fetch digger
dub run digger -- build "master + druntime#2332"

@PetarKirov PetarKirov mentioned this pull request Oct 18, 2018
Closed
jacob-carlborg
jacob-carlborg reviewed Oct 18, 2018
View reviewed changes
src/core/memory.d Outdated
/**
* Aggregation of current profile information
*/
static struct ProfileStats {
Copy link
Contributor

@jacob-carlborg jacob-carlborg Oct 18, 2018

Choose a reason for hiding this comment

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

Curly braces on their own lines.

@marcioapm
Copy link
Contributor

marcioapm commented Dec 7, 2018

Can anybody help getting this included in next release? Build seems to be failing for unrelated reasons. Would a rebase help?

@thewilsonator
Copy link
Contributor

thewilsonator commented Dec 12, 2018

@ZombineDev ping.

@PetarKirov
Copy link
Member Author

PetarKirov commented Dec 12, 2018

@thewilsonator In the short term, I don't think I'll be able to dedicate the time this PR deserves to get through the finish line. Can you adopt it?

@thewilsonator thewilsonator force-pushed the gc-runtime-profile-stats branch from 6f20469 to f13a119 Compare December 12, 2018 11:05
@thewilsonator
Copy link
Contributor

thewilsonator commented Dec 12, 2018

rebased, we'll see what happens.

@rainers
Copy link
Member

rainers commented Dec 22, 2018

I've pushed an update to mak/SRC and mak/COPY for missing files. It seems the compiler adds a ModuleInfo reference to a module that is not on the command line.

@rainers rainers force-pushed the gc-runtime-profile-stats branch 2 times, most recently from 124c3eb to d186561 Compare December 22, 2018 20:28
@PetarKirov PetarKirov requested a review from jmdavis as a code owner December 22, 2018 20:53
@rainers
Copy link
Member

rainers commented Dec 23, 2018

I guess cleaning up the SRC and COPY files should be separated into another PR...

thewilsonator
thewilsonator approved these changes Dec 24, 2018
View reviewed changes
Copy link
Contributor

@thewilsonator thewilsonator left a comment

Choose a reason for hiding this comment

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

LGTM, modulo the brace on new line

@rainers rainers added the GC garbage collector label Dec 24, 2018
@thewilsonator thewilsonator added the 72h no objection -> merge The PR will be merged if there are no objections raised. label Jan 3, 2019
@thewilsonator
Copy link
Contributor

thewilsonator commented Jan 4, 2019

Please squash

@marcioapm @rainers
fix issue 16377 - Add access to GC runtime profile stats
81732af 
synchronize mak/SRC and mak/COPY
fix module name of core.sys.freebsd.pthread_np
@rainers rainers force-pushed the gc-runtime-profile-stats branch from 5aaa42c to 81732af Compare January 4, 2019 10:01
@dlang-bot dlang-bot added the Enhancement New functionality label Jan 4, 2019
@rainers
Copy link
Member

rainers commented Jan 4, 2019

Rebased, squashed and added a micro-test.

BTW: the original PR was already approved by Andrei: #2155 (review)

@thewilsonator thewilsonator added auto-merge and removed 72h no objection -> merge The PR will be merged if there are no objections raised. labels Jan 4, 2019
@dlang-bot dlang-bot merged commit 29f495b into dlang:master Jan 4, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@andralex andralex Awaiting requested review from andralex

@DmitryOlshansky DmitryOlshansky Awaiting requested review from DmitryOlshansky

@MartinNowak MartinNowak Awaiting requested review from MartinNowak

@rainers rainers Awaiting requested review from rainers

@wilzbach wilzbach Awaiting requested review from wilzbach

@jmdavis jmdavis Awaiting requested review from jmdavis

2 more reviewers

@jacob-carlborg jacob-carlborg jacob-carlborg left review comments

@thewilsonator thewilsonator thewilsonator approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

Enhancement New functionality GC garbage collector

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@PetarKirov @dlang-bot @marcioapm @thewilsonator @rainers @jacob-carlborg