Export getThreadId from GHC.Conc.Sync

[bgamari]

Currently GHC.Conc.Sync.ThreadId is nearly entirely opaque, supporting only Eq, Ord, and Show. Such a restrictive interface is quite limiting and precludes the user providing instances like Hashable without relying on GHC's internals (e.g. see https://hackage-search.serokell.io/?q=rts_getThreadId). Moreover, this is quite silly as GHC already associates a unique integer identifier with each thread.

I propose exporting the following from GHC.Conc.Sync:

fromThreadId :: ThreadId -> Word64

This will expose the thread identifier to the user.

See also: GHC #22700

[nomeata]

Is that a good name? I was assuming a function of that name to have type IO ThreadId maybe.

threasIdToInt maybe, as ugly as that is?

[treeowl]

I agree with @nomeata .

[bgamari]

Sure, I have no strong opinion about the name. I have updated the proposal with this suggestion.

[arybczak]

The function should have a type ThreadId -> Int64, because rts_getThreadId returns a CULLong.

Also, for the risk of unnecessary bikeshedding, I had to import this in one in one of my projects and used the name weakThreadId because it kinda is - it's a unique thread identifier, but holding onto it (unlike holding onto ThreadId# itself) won't prevent the thread from getting garbage collected if appropriate.

[chessai]

The function should have a type ThreadId -> Int64, because rts_getThreadId returns a CULLong.

Shouldn't that be Word64 then?

[phadej]

Yes. Which makes foreign import rts_getThreadId use the wrong type. It should use Word64 as the types defined are

typedef StgWord64 StgThreadID;
StgThreadID rts_getThreadId                  (StgPtr tso);

So it seems that all code on Hackage importing rts_getThreadId including base is using a wrong type. It's accidental that CULLong is Word64 on all platforms GHC supports :)

[Bodigrim]

See also: GHC #22700

@bgamari this is a wrong ticket, I think.

[bgamari]

The function should have a type ThreadId -> Int64, because rts_getThreadId returns a CULLong.

Yes, a good point. I will update the proposal.

@bgamari this is a wrong ticket, I think.

Fixed.

[bgamari]

On @phadej's request, I have further updated the proposal such that threadIdToInt returns a Word64, reflecting the fact that the RTS's internal representation is unsigned.

However, this now introduces the awkward fact that threadIdToInt doesn't return an Int. I suppose we can explain this away by claiming that "Int" is in fact short for "Integer" and doesn't refer to the Int type. However, it does seem a bit odd. Other names that come to mind include:

• getThreadNumber
• threadIdToWord
• getThreadIdentifier
• threadId
• threadIdId

[arybczak]

To be fair, all of these are a bit confusing. It's somewhat unfortunate since at this point you already have the ThreadId that supposedly represents the identifier of a thread. Maybe rtsThreadId (or rtsThreadIdentifier)? Then the name at least points to the origin of the number.

[mpickering]

Please can we have a resolution on this @haskell/core-language-committee ?

[Bodigrim]

@mpickering #117 (comment) looks to me like an invitation for discussion, not the final proposal for a vote.

[bgamari]

@Bodigrim, to my mind the proposal is done and I think the proposed naming is acceptable. I presented the other alternatives to spur discussion but given that there aren't any strong opinions on the matter I think it would be fine to put this to a vote.

[Bodigrim]

CLC is currently on pause anyway until new members are elected, which will hopefully happen somewhere next week.

[phadej]

Sorry for bringing more unwelcome news, but I was reading base-4.18.0.0 changelog, and there are few entries mentioning GHC.Conc.Sync:

• GHC.Conc.Sync.listThreads was added, allowing the user to list the threads (both running and blocked) of the program.
• GHC.Conc.Sync.labelThreadByteArray# was added, allowing the user to specify a thread label by way of a ByteArray# containing a UTF-8-encoded string. The old GHC.Conc.Sync.labelThread is now implemented in terms of this function.
• GHC.Conc.Sync.threadLabel was added, allowing the user to query the label of a given ThreadId.

Which are not unlike this issue.

---

OTOH, these are good news also, as it seems to be the only entries on the base-4.18.0.0 changelog without CLC Proposal, all others have them, except

• Eq Generically and Functor (,,,,,) which I guess are consequences of Proposal: Relax instances for Functor combinators; put superclasses on <class>1 to make less-breaking  #10 and Add quantified superclass to Bifunctor #91
• changes to GHC.Stats and GHC.Weak which are a borderline cases, would these require CLC proposals too? I hope that CLC, GHC Steering Committee?, HF? figure out whether the do or not (the criteria), so we won't have to ponder similar stuff for base-4.19.0.0.

[Bodigrim]

Sorry for bringing more unwelcome news, but I was reading base-4.18.0.0 changelog, and there are few entries mentioning GHC.Conc.Sync

Thanks for digging it out, it is not unwelcome at all. We should have done a better job with regards to communication and monitoring last year. Reverting the changes and ramming through a proposal in a week or so left before 9.6 RC will be a farce, not a public scrutiny. Given that GHC.Conc.Sync is a relatively obscure corner I am inclined to think that it is in the public interest to let them be as is, even while I do not enjoy it. Same for GHC.Weak.Finalize.

GHC.Stats is a worse example: not only extending a record is a breaking change, but it is also misattributed to base-4.17: https://ghc.gitlab.haskell.org/ghc/doc/libraries/base-4.18.0.0/GHC-Stats.html#t:GCDetails. This needs to be fixed before base-4.18 released.

---

Back to the topic, threadIdToInt :: ThreadId -> Word64 looks wrong to me. threadIdToWord64 is certainly better.

[Bodigrim]

The proposal is to export rts_getThreadId from GHC.Conc.Sync:

foreign import ccall unsafe "rts_getThreadId" getThreadId :: ThreadId# -> CULLong

Strictly speaking, CULLong type here is a mistake, because according to C sources it should be Word64. These types are the same on all platforms GHC supports, but we should take a chance to rectify this as well.

Now, the hardest question of all: naming. It might be suboptimal for a public function of type ThreadId# -> Word64 to be called getThreadId as it is at the moment. After all, it takes ThreadId#, not returns. Here are options so far:

• threadIdToInt,
• threadIdToWord64,
• getThreadId,
• getThreadNumber,
• threadIdToWord,
• getThreadIdentifier,
• threadId,
• threadIdId.

Dear CLC members, let's help the proposer to choose a name. This is not a vote, just non-binding opinions. Obviously, if you are against exporting this function under any name, please say so as well.

@chshersh @mixphix @angerman @tomjaguarpaw @parsonsmatt @hasufell.

[hasufell]

fromThreadId

This way we don't need to rename in case the API breaks with a different type (e.g. another newtype over Word64 or god knows what).

[angerman]

I like @hasufell's suggestion. I find it's the clearest. ToInt feels certainly wrong. Especially as it does not align with the signature.

[mixphix]

I agree on fromThreadId. Prefixing with get feels monadic, and the threadIdToX becomes either redundant or wrong if X changes for some reason.

[Bodigrim]

@bgamari are you happy with fromThreadId :: ThreadId -> Word64? If you update the proposal, we'll be able to vote.

[Bodigrim]

@bgamari could you please update the proposal?

[bgamari]

I have updated the proposal to reflect the fromThreadId name.

[bgamari]

One remaining question is whether fromThreadId should also be exported from Control.Concurrent as well since GHC.Conc.Sync is currently a grab-bag of various GHC-specific implementation details.

[Bodigrim]

To a certain extent fromThreadId :: ThreadId -> Word is a GHC-specific implementation detail, another compiler with another RTS could be doing it differently. If need arises, it can be re-exported from Control.Concurrent later. But I'm personally fine either way.

Unless someone feels strongly that re-exporting fromThreadId from Control.Concurrent is a tangible improvement right now, I'll trigger a vote on the proposal as is soon.

[bgamari]

An immediate vote is fine with me.

[Bodigrim]

Dear CLC members, any non-binding opinions on this?

[chshersh]

I'm happy with the name of the function, its type and it being exported from GHC.Conc.Sync. I would be fine with reexporting from Control.Concurrent as well. But I personally don't have problems with importing GHC.Conc.Sync for this function, and I don't think it will be used that often by beginners to make imports more beginner-friendly. Besides, the GHC.* import does suggest that this is how GHC represents ThreadId, and its implementation is tied to the GHC internals.

However, the documentation for ThreadId could mention this helpful function regardless of reexport.

[Bodigrim]

Dear CLC members, let's vote on the proposal to add GHC.Conc.Sync.fromThreadId :: ThreadId -> Word64 as detailed in https://gitlab.haskell.org/ghc/ghc/-/merge_requests/10176/diffs. The motivation is to provide a "blessed" way to convert ThreadId to an integer instead of FFI'ng into RTS headers as people do now with rts_getThreadId (https://hackage-search.serokell.io/?q=rts_getThreadId).

@chshersh @tomjaguarpaw @hasufell @angerman @parsonsmatt @mixphix

---

+1 from me.

[hasufell]

+1

[angerman]

+1

[chshersh]

+1

[parsonsmatt]

+1

[Bodigrim]

Thanks all, 5 votes out of 7 are enough to approve.