Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Expose --no-automatic-time-samples in GHC.RTS.Flags API #243

Closed
mpickering opened this issue Jan 17, 2024 · 10 comments
Closed

Expose --no-automatic-time-samples in GHC.RTS.Flags API #243

mpickering opened this issue Jan 17, 2024 · 10 comments
Labels
approved Approved by CLC vote

Comments

@mpickering
Copy link

GHC MR: https://gitlab.haskell.org/ghc/ghc/-/merge_requests/11916

In GHC MR !11424 a new RTS flag was added --no-automatic-time-samples.

Therefore we should also modify GHC.RTS.Flags in the same manner so that users can query whether this option is set from their programs, and keep GHC.RTS.Flags up to date with the implemented RTS flags.

@mpickering
Copy link
Author

Is there anything which can be done to move this issue forwards? It seems that we should keep GHC.RTS.Flags up to date with the flags implemented by the RTS.

@Bodigrim
Copy link
Collaborator

Bodigrim commented Feb 1, 2024

Technically adding a record field is a breaking change, but given how obscure GHC.RTS.Flags is...

ProfFlags are used only in handful of places throughout entire Hackage: https://hackage-search.serokell.io/?q=%5CbProfFlags. None of them will be broken by the change.

Thus I'm content to vote in favor. Dear CLC members, any more opinions before we vote?

@tomjaguarpaw
Copy link
Member

It seems fine to me. If GHC's interface changes then we need to make the corresponding change to anything that represents its interface. Ultimately though, given that this is a GHC-only concern, I'd love to see this functionality migrated into a package under the sole purview of GHC HQ.

@Bodigrim
Copy link
Collaborator

Dear CLC members, let's vote on the proposal to extend GHC.RTS.Flags to reflect faithfully RTS flags, including recently added one --no-automatic-time-samples. The MR is available at https://gitlab.haskell.org/ghc/ghc/-/merge_requests/11916 and boils down to extending data ProfFlags with startTimeProfileAtStartup :: Bool.

@tomjaguarpaw @mixphix @hasufell @velveteer @angerman @parsonsmatt


+1 from me.

@Bodigrim Bodigrim added the vote-in-progress The CLC vote is in progress label Feb 11, 2024
@tomjaguarpaw
Copy link
Member

tomjaguarpaw commented Feb 11, 2024

+1


I risk sounding monotonous and saying things redundantly, but I'll reiterate anyway that these kinds of GHC configuration types should be in a GHC-internal module in the future.

EDIT: Actually I shouldn't say "internal". It's up to GHC HQ whether they expose these from a package that they intend to keep stable (I encourage them to do so), it just shouldn't be base, because they are deeply linked to GHC implementation details.

@velveteer
Copy link
Contributor

+1

2 similar comments
@parsonsmatt
Copy link

+1

@angerman
Copy link

+1

@mpickering
Copy link
Author

@tomjaguarpaw I think we are in agreement there, and if you or another CLC member would like to open a proposal to move GHC.RTS.Flags out of base then I think GHC developers would support that.

@Bodigrim
Copy link
Collaborator

Thanks all, that's enough votes to approve.

@Bodigrim Bodigrim added approved Approved by CLC vote and removed vote-in-progress The CLC vote is in progress labels Feb 12, 2024
hubot pushed a commit to ghc/ghc that referenced this issue Feb 14, 2024
This patch builds on 5077416 and
modifies the base API to reflect the new RTS flag.

CLC proposal #243 - haskell/core-libraries-committee#243

Fixes #24337
hubot pushed a commit to ghc/ghc that referenced this issue Feb 15, 2024
This patch builds on 5077416 and
modifies the base API to reflect the new RTS flag.

CLC proposal #243 - haskell/core-libraries-committee#243

Fixes #24337
hubot pushed a commit to ghc/ghc that referenced this issue Feb 15, 2024
This patch builds on 5077416 and
modifies the base API to reflect the new RTS flag.

CLC proposal #243 - haskell/core-libraries-committee#243

Fixes #24337
hubot pushed a commit to ghc/ghc that referenced this issue Feb 15, 2024
This patch builds on 5077416 and
modifies the base API to reflect the new RTS flag.

CLC proposal #243 - haskell/core-libraries-committee#243

Fixes #24337
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Approved by CLC vote
Projects
None yet
Development

No branches or pull requests

6 participants