Unique as special boxing type & hidden constructors

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
17 messages Options
Reply | Threaded
Open this post in threaded view
|

Unique as special boxing type & hidden constructors

p.k.f.holzenspies@utwente.nl
Dear all,


I'm working with Alan to instantiate everything for Data.Data, so that we can do better SYB-traversals (which should also help newcomers significantly to get into the GHC code base). Alan's looking at the AST types, I'm looking at the basic types in the compiler.


Right now, I'm looking at Unique and two questions come up:


> data Unique = MkUnique FastInt


1) As someone already commented: Is there a specific reason (other than history) that this isn't simply a newtype around an Int? If we're boxing anyway, we may as well use the default Int boxing and newtype-coerce to the specific purpose of Unique, no?


2) As a general question for GHC hacking style; what is the reason for hiding the constructors in the first place?


I understand about abstraction and there are reasons for hiding, but there's a "public GHC API" and then there are all these modules that people can import at their own peril. Nothing is guaranteed about their consistency from version to version of GHC. I don't really see the point about hiding constructors (getting in the way of automatically deriving things) and then giving extra functions like (in the case of Unique):


> getKeyFastInt (MkUnique x) = x

> mkUniqueGrimily x = MkUnique (iUnbox x)


I would propose to just make Unique a newtype for an Int and making the constructor visible.


Regards,

Philip

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.haskell.org/pipermail/ghc-devs/attachments/20140815/d8dbd8b6/attachment.html>

Reply | Threaded
Open this post in threaded view
|

Unique as special boxing type & hidden constructors

Edward Z. Yang
The definition dates back to 1996, so it seems plausible that
newtype is the way to go now.

Edward

Excerpts from p.k.f.holzenspies's message of 2014-08-15 11:52:47 +0100:

> Dear all,
>
>
> I'm working with Alan to instantiate everything for Data.Data, so that we can do better SYB-traversals (which should also help newcomers significantly to get into the GHC code base). Alan's looking at the AST types, I'm looking at the basic types in the compiler.
>
> Right now, I'm looking at Unique and two questions come up:
>
> > data Unique = MkUnique FastInt
>
>
> 1) As someone already commented: Is there a specific reason (other than history) that this isn't simply a newtype around an Int? If we're boxing anyway, we may as well use the default Int boxing and newtype-coerce to the specific purpose of Unique, no?
>
>
> 2) As a general question for GHC hacking style; what is the reason for hiding the constructors in the first place?
>
> I understand about abstraction and there are reasons for hiding, but there's a "public GHC API" and then there are all these modules that people can import at their own peril. Nothing is guaranteed about their consistency from version to version of GHC. I don't really see the point about hiding constructors (getting in the way of automatically deriving things) and then giving extra functions like (in the case of Unique):
>
> > getKeyFastInt (MkUnique x) = x
>
> > mkUniqueGrimily x = MkUnique (iUnbox x)
>
>
> I would propose to just make Unique a newtype for an Int and making the constructor visible.
>
>
> Regards,
>
> Philip

Reply | Threaded
Open this post in threaded view
|

Unique as special boxing type & hidden constructors

Simon Peyton Jones
In reply to this post by p.k.f.holzenspies@utwente.nl
Re (1) I think this is a historical.  A newtype wrapping an Int should be fine.  I'd be ok with that change.

Re (2), I think your question is: why does module Unique export the data type Unique abstractly, rather than exporting both the data type and its constructor.  No deep reason here, but it guarantees that you can only *make* a unique from an Int by calling 'mkUniqueGrimily', which signals clearly that something fishy is going on.  And rightly so!

Simon

From: ghc-devs [mailto:ghc-devs-bounces at haskell.org] On Behalf Of p.k.f.holzenspies at utwente.nl
Sent: 15 August 2014 11:53
To: ghc-devs at haskell.org
Subject: Unique as special boxing type & hidden constructors


Dear all,



I'm working with Alan to instantiate everything for Data.Data, so that we can do better SYB-traversals (which should also help newcomers significantly to get into the GHC code base). Alan's looking at the AST types, I'm looking at the basic types in the compiler.



Right now, I'm looking at Unique and two questions come up:



> data Unique = MkUnique FastInt



1) As someone already commented: Is there a specific reason (other than history) that this isn't simply a newtype around an Int? If we're boxing anyway, we may as well use the default Int boxing and newtype-coerce to the specific purpose of Unique, no?



2) As a general question for GHC hacking style; what is the reason for hiding the constructors in the first place?



I understand about abstraction and there are reasons for hiding, but there's a "public GHC API" and then there are all these modules that people can import at their own peril. Nothing is guaranteed about their consistency from version to version of GHC. I don't really see the point about hiding constructors (getting in the way of automatically deriving things) and then giving extra functions like (in the case of Unique):



> getKeyFastInt (MkUnique x) = x

> mkUniqueGrimily x = MkUnique (iUnbox x)



I would propose to just make Unique a newtype for an Int and making the constructor visible.

Regards,

Philip


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.haskell.org/pipermail/ghc-devs/attachments/20140817/e318bb20/attachment.html>

Reply | Threaded
Open this post in threaded view
|

Unique as special boxing type & hidden constructors

p.k.f.holzenspies@utwente.nl
Dear Simon, et al,


Looking at Unique, there are a few more design choices that may be outdated, and since I'm polishing things now, anyway, I figured I could update it on more fronts.


1) There is a #ifdef define(__GLASGOW_HASKELL__), which confused me somewhat. Similar things occur elsewhere in the code. Isn't the assumption that GHC is being used? Is this old portability stuff that may be removed?


2) Uniques are produced from a Char and an Int. The function to build Uniques (mkUnique) is not exported, according to the comments, so as to see all characters used. Access to these different "classes" of Uniques is given through specialised mkXXXUnique functions. Does anyone have a problem with something like:


> data UniqueClass

>   = UniqDesugarer

>   | UniqAbsCFlattener

>   | UniqSimplStg

>   | UniqNativeCodeGen

>   ...


and a public (i.e. exported) function:


> mkUnique :: UniqueClass -> Int -> Unique


? The benefit of this would be to have more (to my taste) self-documenting code and a greater chance that documentation is updated (the list of "unique supply characters" in the comments is currently outdated).


3) Is there a reason for having functions implementing class-methods to be exported? In the case of Unique, there is pprUnique and:


> instance Outputable Unique where

>   ppr = pprUnique


Here pprUnique is exported and it is used in quite a few places where it's argument is unambiguously a Unique (so it's not to force the type) *and* "ppr" is used for all kinds of other types. I'm assuming this is an old choice making things marginally faster, but I would say cleaning up the API / namespace would now outweigh this margin.

?

I will also be adding Haddock-comments, so when this is done, a review would be most welcome (I'll also be doing some similar transformations to other long-since-untouched-code).


Regards,

Philip









________________________________
Van: Simon Peyton Jones <simonpj at microsoft.com>
Verzonden: maandag 18 augustus 2014 00:11
Aan: Holzenspies, P.K.F. (EWI); ghc-devs at haskell.org
Onderwerp: RE: Unique as special boxing type & hidden constructors

Re (1) I think this is a historical.  A newtype wrapping an Int should be fine.  I?d be ok with that change.

Re (2), I think your question is: why does module Unique export the data type Unique abstractly, rather than exporting both the data type and its constructor.  No deep reason here, but it guarantees that you can only *make* a unique from an Int by calling ?mkUniqueGrimily?, which signals clearly that something fishy is going on.  And rightly so!

Simon

From: ghc-devs [mailto:ghc-devs-bounces at haskell.org] On Behalf Of p.k.f.holzenspies at utwente.nl
Sent: 15 August 2014 11:53
To: ghc-devs at haskell.org
Subject: Unique as special boxing type & hidden constructors


Dear all,



I'm working with Alan to instantiate everything for Data.Data, so that we can do better SYB-traversals (which should also help newcomers significantly to get into the GHC code base). Alan's looking at the AST types, I'm looking at the basic types in the compiler.



Right now, I'm looking at Unique and two questions come up:



> data Unique = MkUnique FastInt



1) As someone already commented: Is there a specific reason (other than history) that this isn't simply a newtype around an Int? If we're boxing anyway, we may as well use the default Int boxing and newtype-coerce to the specific purpose of Unique, no?



2) As a general question for GHC hacking style; what is the reason for hiding the constructors in the first place?



I understand about abstraction and there are reasons for hiding, but there's a "public GHC API" and then there are all these modules that people can import at their own peril. Nothing is guaranteed about their consistency from version to version of GHC. I don't really see the point about hiding constructors (getting in the way of automatically deriving things) and then giving extra functions like (in the case of Unique):



> getKeyFastInt (MkUnique x) = x

> mkUniqueGrimily x = MkUnique (iUnbox x)



I would propose to just make Unique a newtype for an Int and making the constructor visible.

Regards,

Philip


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.haskell.org/pipermail/ghc-devs/attachments/20140818/58e853c6/attachment-0001.html>

Reply | Threaded
Open this post in threaded view
|

Unique as special boxing type & hidden constructors

p.k.f.holzenspies@utwente.nl
PS.


Unique also looks like a case where Ints are used and (>= 0) is asserted. Can these cases be converted to Word as per earlier discussions?




________________________________
Van: p.k.f.holzenspies at utwente.nl <p.k.f.holzenspies at utwente.nl>
Verzonden: maandag 18 augustus 2014 15:49
Aan: simonpj at microsoft.com; ghc-devs at haskell.org
Onderwerp: RE: Unique as special boxing type & hidden constructors


Dear Simon, et al,


Looking at Unique, there are a few more design choices that may be outdated, and since I'm polishing things now, anyway, I figured I could update it on more fronts.


1) There is a #ifdef define(__GLASGOW_HASKELL__), which confused me somewhat. Similar things occur elsewhere in the code. Isn't the assumption that GHC is being used? Is this old portability stuff that may be removed?


2) Uniques are produced from a Char and an Int. The function to build Uniques (mkUnique) is not exported, according to the comments, so as to see all characters used. Access to these different "classes" of Uniques is given through specialised mkXXXUnique functions. Does anyone have a problem with something like:


> data UniqueClass

>   = UniqDesugarer

>   | UniqAbsCFlattener

>   | UniqSimplStg

>   | UniqNativeCodeGen

>   ...


and a public (i.e. exported) function:


> mkUnique :: UniqueClass -> Int -> Unique


? The benefit of this would be to have more (to my taste) self-documenting code and a greater chance that documentation is updated (the list of "unique supply characters" in the comments is currently outdated).


3) Is there a reason for having functions implementing class-methods to be exported? In the case of Unique, there is pprUnique and:


> instance Outputable Unique where

>   ppr = pprUnique


Here pprUnique is exported and it is used in quite a few places where it's argument is unambiguously a Unique (so it's not to force the type) *and* "ppr" is used for all kinds of other types. I'm assuming this is an old choice making things marginally faster, but I would say cleaning up the API / namespace would now outweigh this margin.

?

I will also be adding Haddock-comments, so when this is done, a review would be most welcome (I'll also be doing some similar transformations to other long-since-untouched-code).


Regards,

Philip









________________________________
Van: Simon Peyton Jones <simonpj at microsoft.com>
Verzonden: maandag 18 augustus 2014 00:11
Aan: Holzenspies, P.K.F. (EWI); ghc-devs at haskell.org
Onderwerp: RE: Unique as special boxing type & hidden constructors

Re (1) I think this is a historical.  A newtype wrapping an Int should be fine.  I?d be ok with that change.

Re (2), I think your question is: why does module Unique export the data type Unique abstractly, rather than exporting both the data type and its constructor.  No deep reason here, but it guarantees that you can only *make* a unique from an Int by calling ?mkUniqueGrimily?, which signals clearly that something fishy is going on.  And rightly so!

Simon

From: ghc-devs [mailto:ghc-devs-bounces at haskell.org] On Behalf Of p.k.f.holzenspies at utwente.nl
Sent: 15 August 2014 11:53
To: ghc-devs at haskell.org
Subject: Unique as special boxing type & hidden constructors


Dear all,



I'm working with Alan to instantiate everything for Data.Data, so that we can do better SYB-traversals (which should also help newcomers significantly to get into the GHC code base). Alan's looking at the AST types, I'm looking at the basic types in the compiler.



Right now, I'm looking at Unique and two questions come up:



> data Unique = MkUnique FastInt



1) As someone already commented: Is there a specific reason (other than history) that this isn't simply a newtype around an Int? If we're boxing anyway, we may as well use the default Int boxing and newtype-coerce to the specific purpose of Unique, no?



2) As a general question for GHC hacking style; what is the reason for hiding the constructors in the first place?



I understand about abstraction and there are reasons for hiding, but there's a "public GHC API" and then there are all these modules that people can import at their own peril. Nothing is guaranteed about their consistency from version to version of GHC. I don't really see the point about hiding constructors (getting in the way of automatically deriving things) and then giving extra functions like (in the case of Unique):



> getKeyFastInt (MkUnique x) = x

> mkUniqueGrimily x = MkUnique (iUnbox x)



I would propose to just make Unique a newtype for an Int and making the constructor visible.

Regards,

Philip


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.haskell.org/pipermail/ghc-devs/attachments/20140818/ba3c6392/attachment-0001.html>

Reply | Threaded
Open this post in threaded view
|

Unique as special boxing type & hidden constructors

Simon Peyton Jones
In reply to this post by p.k.f.holzenspies@utwente.nl
1) There is a #ifdef define(__GLASGOW_HASKELL__), which confused me somewhat. Similar things occur elsewhere in the code. Isn't the assumption that GHC is being used? Is this old portability stuff that may be removed?

I think so, unless others yell to the contrary.


2) Uniques are produced from a Char and an Int. The function to build Uniques (mkUnique) is not exported, according to the comments, so as to see all characters used. Access to these different "classes" of Uniques is given through specialised mkXXXUnique functions. Does anyone have a problem with something like:



> data UniqueClass

>   = UniqDesugarer

>   | UniqAbsCFlattener

>   | UniqSimplStg

>   | UniqNativeCodeGen

>   ...

OK by me


3) Is there a reason for having functions implementing class-methods to be exported? In the case of Unique, there is pprUnique and:

> instance Outputable Unique where

>   ppr = pprUnique

Please don?t change this.  If you want to change how pretty-printing of uniques works, and want to find all the call sites of pprUnique, it?s FAR easier to grep for pprUnique than to search for  all calls of ppr, and work out which are at type Unique!

(In my view) it?s usually much better not to use type classes unless you actually need overloading.

Simon

From: p.k.f.holzenspies at utwente.nl [mailto:p.k.f.holzenspies at utwente.nl]
Sent: 18 August 2014 14:50
To: Simon Peyton Jones; ghc-devs at haskell.org
Subject: RE: Unique as special boxing type & hidden constructors


Dear Simon, et al,



Looking at Unique, there are a few more design choices that may be outdated, and since I'm polishing things now, anyway, I figured I could update it on more fronts.



1) There is a #ifdef define(__GLASGOW_HASKELL__), which confused me somewhat. Similar things occur elsewhere in the code. Isn't the assumption that GHC is being used? Is this old portability stuff that may be removed?



2) Uniques are produced from a Char and an Int. The function to build Uniques (mkUnique) is not exported, according to the comments, so as to see all characters used. Access to these different "classes" of Uniques is given through specialised mkXXXUnique functions. Does anyone have a problem with something like:



> data UniqueClass

>   = UniqDesugarer

>   | UniqAbsCFlattener

>   | UniqSimplStg

>   | UniqNativeCodeGen

>   ...



and a public (i.e. exported) function:



> mkUnique :: UniqueClass -> Int -> Unique



? The benefit of this would be to have more (to my taste) self-documenting code and a greater chance that documentation is updated (the list of "unique supply characters" in the comments is currently outdated).



3) Is there a reason for having functions implementing class-methods to be exported? In the case of Unique, there is pprUnique and:



> instance Outputable Unique where

>   ppr = pprUnique



Here pprUnique is exported and it is used in quite a few places where it's argument is unambiguously a Unique (so it's not to force the type) *and* "ppr" is used for all kinds of other types. I'm assuming this is an old choice making things marginally faster, but I would say cleaning up the API / namespace would now outweigh this margin.

?

I will also be adding Haddock-comments, so when this is done, a review would be most welcome (I'll also be doing some similar transformations to other long-since-untouched-code).



Regards,

Philip

















________________________________
Van: Simon Peyton Jones <simonpj at microsoft.com<mailto:simonpj at microsoft.com>>
Verzonden: maandag 18 augustus 2014 00:11
Aan: Holzenspies, P.K.F. (EWI); ghc-devs at haskell.org<mailto:ghc-devs at haskell.org>
Onderwerp: RE: Unique as special boxing type & hidden constructors

Re (1) I think this is a historical.  A newtype wrapping an Int should be fine.  I?d be ok with that change.

Re (2), I think your question is: why does module Unique export the data type Unique abstractly, rather than exporting both the data type and its constructor.  No deep reason here, but it guarantees that you can only *make* a unique from an Int by calling ?mkUniqueGrimily?, which signals clearly that something fishy is going on.  And rightly so!

Simon

From: ghc-devs [mailto:ghc-devs-bounces at haskell.org] On Behalf Of p.k.f.holzenspies at utwente.nl<mailto:p.k.f.holzenspies at utwente.nl>
Sent: 15 August 2014 11:53
To: ghc-devs at haskell.org<mailto:ghc-devs at haskell.org>
Subject: Unique as special boxing type & hidden constructors


Dear all,



I'm working with Alan to instantiate everything for Data.Data, so that we can do better SYB-traversals (which should also help newcomers significantly to get into the GHC code base). Alan's looking at the AST types, I'm looking at the basic types in the compiler.



Right now, I'm looking at Unique and two questions come up:



> data Unique = MkUnique FastInt



1) As someone already commented: Is there a specific reason (other than history) that this isn't simply a newtype around an Int? If we're boxing anyway, we may as well use the default Int boxing and newtype-coerce to the specific purpose of Unique, no?



2) As a general question for GHC hacking style; what is the reason for hiding the constructors in the first place?



I understand about abstraction and there are reasons for hiding, but there's a "public GHC API" and then there are all these modules that people can import at their own peril. Nothing is guaranteed about their consistency from version to version of GHC. I don't really see the point about hiding constructors (getting in the way of automatically deriving things) and then giving extra functions like (in the case of Unique):



> getKeyFastInt (MkUnique x) = x

> mkUniqueGrimily x = MkUnique (iUnbox x)



I would propose to just make Unique a newtype for an Int and making the constructor visible.

Regards,

Philip


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.haskell.org/pipermail/ghc-devs/attachments/20140818/fedd9800/attachment-0001.html>

Reply | Threaded
Open this post in threaded view
|

Unique as special boxing type & hidden constructors

p.k.f.holzenspies@utwente.nl
Dear Simon, et al,


I seem to recall that the Unique(Supply) was an issue in parallelising GHC itself. There's a comment in the code (signed JSM) that there aren't any 64-bit bugs, if we have at least 32-bits for Ints and Chars fit in 8 characters. Then, there's bitmasks like 0x00FFFFFF to separate the "Int-part" from the "Char-part".


I was wondering; if we move Uniques to 64 bits, but use the top 16 (instead of the current 8) for *both* the tag (currently a Char, soon an sum-type) and the threadId of the supplying thread of a Unique, would that help?


Regards,

Philip





________________________________
From: Simon Peyton Jones <simonpj at microsoft.com>
Sent: 18 August 2014 23:29
To: Holzenspies, P.K.F. (EWI); ghc-devs at haskell.org
Subject: RE: Unique as special boxing type & hidden constructors


1) There is a #ifdef define(__GLASGOW_HASKELL__), which confused me somewhat. Similar things occur elsewhere in the code. Isn't the assumption that GHC is being used? Is this old portability stuff that may be removed?

I think so, unless others yell to the contrary.


2) Uniques are produced from a Char and an Int. The function to build Uniques (mkUnique) is not exported, according to the comments, so as to see all characters used. Access to these different "classes" of Uniques is given through specialised mkXXXUnique functions. Does anyone have a problem with something like:



> data UniqueClass

>   = UniqDesugarer

>   | UniqAbsCFlattener

>   | UniqSimplStg

>   | UniqNativeCodeGen

>   ...

OK by me


3) Is there a reason for having functions implementing class-methods to be exported? In the case of Unique, there is pprUnique and:

> instance Outputable Unique where

>   ppr = pprUnique

Please don?t change this.  If you want to change how pretty-printing of uniques works, and want to find all the call sites of pprUnique, it?s FAR easier to grep for pprUnique than to search for  all calls of ppr, and work out which are at type Unique!

(In my view) it?s usually much better not to use type classes unless you actually need overloading.

Simon

From: p.k.f.holzenspies at utwente.nl [mailto:p.k.f.holzenspies at utwente.nl]
Sent: 18 August 2014 14:50
To: Simon Peyton Jones; ghc-devs at haskell.org
Subject: RE: Unique as special boxing type & hidden constructors


Dear Simon, et al,



Looking at Unique, there are a few more design choices that may be outdated, and since I'm polishing things now, anyway, I figured I could update it on more fronts.



1) There is a #ifdef define(__GLASGOW_HASKELL__), which confused me somewhat. Similar things occur elsewhere in the code. Isn't the assumption that GHC is being used? Is this old portability stuff that may be removed?



2) Uniques are produced from a Char and an Int. The function to build Uniques (mkUnique) is not exported, according to the comments, so as to see all characters used. Access to these different "classes" of Uniques is given through specialised mkXXXUnique functions. Does anyone have a problem with something like:



> data UniqueClass

>   = UniqDesugarer

>   | UniqAbsCFlattener

>   | UniqSimplStg

>   | UniqNativeCodeGen

>   ...



and a public (i.e. exported) function:



> mkUnique :: UniqueClass -> Int -> Unique



? The benefit of this would be to have more (to my taste) self-documenting code and a greater chance that documentation is updated (the list of "unique supply characters" in the comments is currently outdated).



3) Is there a reason for having functions implementing class-methods to be exported? In the case of Unique, there is pprUnique and:



> instance Outputable Unique where

>   ppr = pprUnique



Here pprUnique is exported and it is used in quite a few places where it's argument is unambiguously a Unique (so it's not to force the type) *and* "ppr" is used for all kinds of other types. I'm assuming this is an old choice making things marginally faster, but I would say cleaning up the API / namespace would now outweigh this margin.

?

I will also be adding Haddock-comments, so when this is done, a review would be most welcome (I'll also be doing some similar transformations to other long-since-untouched-code).



Regards,

Philip

















________________________________
Van: Simon Peyton Jones <simonpj at microsoft.com<mailto:simonpj at microsoft.com>>
Verzonden: maandag 18 augustus 2014 00:11
Aan: Holzenspies, P.K.F. (EWI); ghc-devs at haskell.org<mailto:ghc-devs at haskell.org>
Onderwerp: RE: Unique as special boxing type & hidden constructors

Re (1) I think this is a historical.  A newtype wrapping an Int should be fine.  I?d be ok with that change.

Re (2), I think your question is: why does module Unique export the data type Unique abstractly, rather than exporting both the data type and its constructor.  No deep reason here, but it guarantees that you can only *make* a unique from an Int by calling ?mkUniqueGrimily?, which signals clearly that something fishy is going on.  And rightly so!

Simon

From: ghc-devs [mailto:ghc-devs-bounces at haskell.org] On Behalf Of p.k.f.holzenspies at utwente.nl<mailto:p.k.f.holzenspies at utwente.nl>
Sent: 15 August 2014 11:53
To: ghc-devs at haskell.org<mailto:ghc-devs at haskell.org>
Subject: Unique as special boxing type & hidden constructors


Dear all,



I'm working with Alan to instantiate everything for Data.Data, so that we can do better SYB-traversals (which should also help newcomers significantly to get into the GHC code base). Alan's looking at the AST types, I'm looking at the basic types in the compiler.



Right now, I'm looking at Unique and two questions come up:



> data Unique = MkUnique FastInt



1) As someone already commented: Is there a specific reason (other than history) that this isn't simply a newtype around an Int? If we're boxing anyway, we may as well use the default Int boxing and newtype-coerce to the specific purpose of Unique, no?



2) As a general question for GHC hacking style; what is the reason for hiding the constructors in the first place?



I understand about abstraction and there are reasons for hiding, but there's a "public GHC API" and then there are all these modules that people can import at their own peril. Nothing is guaranteed about their consistency from version to version of GHC. I don't really see the point about hiding constructors (getting in the way of automatically deriving things) and then giving extra functions like (in the case of Unique):



> getKeyFastInt (MkUnique x) = x

> mkUniqueGrimily x = MkUnique (iUnbox x)



I would propose to just make Unique a newtype for an Int and making the constructor visible.

Regards,

Philip


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.haskell.org/pipermail/ghc-devs/attachments/20140820/82b06407/attachment-0001.html>

Reply | Threaded
Open this post in threaded view
|

Unique as special boxing type & hidden constructors

Simon Peyton Jones
Sounds like a good idea to me.   Would need to think about making sure that it all still worked, somehow, on 32 bit.

S

From: p.k.f.holzenspies at utwente.nl [mailto:p.k.f.holzenspies at utwente.nl]
Sent: 20 August 2014 11:31
To: Simon Peyton Jones; ghc-devs at haskell.org
Subject: RE: Unique as special boxing type & hidden constructors


Dear Simon, et al,



I seem to recall that the Unique(Supply) was an issue in parallelising GHC itself. There's a comment in the code (signed JSM) that there aren't any 64-bit bugs, if we have at least 32-bits for Ints and Chars fit in 8 characters. Then, there's bitmasks like 0x00FFFFFF to separate the "Int-part" from the "Char-part".



I was wondering; if we move Uniques to 64 bits, but use the top 16 (instead of the current 8) for *both* the tag (currently a Char, soon an sum-type) and the threadId of the supplying thread of a Unique, would that help?



Regards,

Philip









________________________________
From: Simon Peyton Jones <simonpj at microsoft.com<mailto:simonpj at microsoft.com>>
Sent: 18 August 2014 23:29
To: Holzenspies, P.K.F. (EWI); ghc-devs at haskell.org<mailto:ghc-devs at haskell.org>
Subject: RE: Unique as special boxing type & hidden constructors


1) There is a #ifdef define(__GLASGOW_HASKELL__), which confused me somewhat. Similar things occur elsewhere in the code. Isn't the assumption that GHC is being used? Is this old portability stuff that may be removed?

I think so, unless others yell to the contrary.


2) Uniques are produced from a Char and an Int. The function to build Uniques (mkUnique) is not exported, according to the comments, so as to see all characters used. Access to these different "classes" of Uniques is given through specialised mkXXXUnique functions. Does anyone have a problem with something like:



> data UniqueClass

>   = UniqDesugarer

>   | UniqAbsCFlattener

>   | UniqSimplStg

>   | UniqNativeCodeGen

>   ...

OK by me


3) Is there a reason for having functions implementing class-methods to be exported? In the case of Unique, there is pprUnique and:

> instance Outputable Unique where

>   ppr = pprUnique

Please don?t change this.  If you want to change how pretty-printing of uniques works, and want to find all the call sites of pprUnique, it?s FAR easier to grep for pprUnique than to search for  all calls of ppr, and work out which are at type Unique!

(In my view) it?s usually much better not to use type classes unless you actually need overloading.

Simon

From: p.k.f.holzenspies at utwente.nl<mailto:p.k.f.holzenspies at utwente.nl> [mailto:p.k.f.holzenspies at utwente.nl]
Sent: 18 August 2014 14:50
To: Simon Peyton Jones; ghc-devs at haskell.org<mailto:ghc-devs at haskell.org>
Subject: RE: Unique as special boxing type & hidden constructors


Dear Simon, et al,



Looking at Unique, there are a few more design choices that may be outdated, and since I'm polishing things now, anyway, I figured I could update it on more fronts.



1) There is a #ifdef define(__GLASGOW_HASKELL__), which confused me somewhat. Similar things occur elsewhere in the code. Isn't the assumption that GHC is being used? Is this old portability stuff that may be removed?



2) Uniques are produced from a Char and an Int. The function to build Uniques (mkUnique) is not exported, according to the comments, so as to see all characters used. Access to these different "classes" of Uniques is given through specialised mkXXXUnique functions. Does anyone have a problem with something like:



> data UniqueClass

>   = UniqDesugarer

>   | UniqAbsCFlattener

>   | UniqSimplStg

>   | UniqNativeCodeGen

>   ...



and a public (i.e. exported) function:



> mkUnique :: UniqueClass -> Int -> Unique



? The benefit of this would be to have more (to my taste) self-documenting code and a greater chance that documentation is updated (the list of "unique supply characters" in the comments is currently outdated).



3) Is there a reason for having functions implementing class-methods to be exported? In the case of Unique, there is pprUnique and:



> instance Outputable Unique where

>   ppr = pprUnique



Here pprUnique is exported and it is used in quite a few places where it's argument is unambiguously a Unique (so it's not to force the type) *and* "ppr" is used for all kinds of other types. I'm assuming this is an old choice making things marginally faster, but I would say cleaning up the API / namespace would now outweigh this margin.

?

I will also be adding Haddock-comments, so when this is done, a review would be most welcome (I'll also be doing some similar transformations to other long-since-untouched-code).



Regards,

Philip

















________________________________
Van: Simon Peyton Jones <simonpj at microsoft.com<mailto:simonpj at microsoft.com>>
Verzonden: maandag 18 augustus 2014 00:11
Aan: Holzenspies, P.K.F. (EWI); ghc-devs at haskell.org<mailto:ghc-devs at haskell.org>
Onderwerp: RE: Unique as special boxing type & hidden constructors

Re (1) I think this is a historical.  A newtype wrapping an Int should be fine.  I?d be ok with that change.

Re (2), I think your question is: why does module Unique export the data type Unique abstractly, rather than exporting both the data type and its constructor.  No deep reason here, but it guarantees that you can only *make* a unique from an Int by calling ?mkUniqueGrimily?, which signals clearly that something fishy is going on.  And rightly so!

Simon

From: ghc-devs [mailto:ghc-devs-bounces at haskell.org] On Behalf Of p.k.f.holzenspies at utwente.nl<mailto:p.k.f.holzenspies at utwente.nl>
Sent: 15 August 2014 11:53
To: ghc-devs at haskell.org<mailto:ghc-devs at haskell.org>
Subject: Unique as special boxing type & hidden constructors


Dear all,



I'm working with Alan to instantiate everything for Data.Data, so that we can do better SYB-traversals (which should also help newcomers significantly to get into the GHC code base). Alan's looking at the AST types, I'm looking at the basic types in the compiler.



Right now, I'm looking at Unique and two questions come up:



> data Unique = MkUnique FastInt



1) As someone already commented: Is there a specific reason (other than history) that this isn't simply a newtype around an Int? If we're boxing anyway, we may as well use the default Int boxing and newtype-coerce to the specific purpose of Unique, no?



2) As a general question for GHC hacking style; what is the reason for hiding the constructors in the first place?



I understand about abstraction and there are reasons for hiding, but there's a "public GHC API" and then there are all these modules that people can import at their own peril. Nothing is guaranteed about their consistency from version to version of GHC. I don't really see the point about hiding constructors (getting in the way of automatically deriving things) and then giving extra functions like (in the case of Unique):



> getKeyFastInt (MkUnique x) = x

> mkUniqueGrimily x = MkUnique (iUnbox x)



I would propose to just make Unique a newtype for an Int and making the constructor visible.

Regards,

Philip


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.haskell.org/pipermail/ghc-devs/attachments/20140820/b8bc0918/attachment-0001.html>

Reply | Threaded
Open this post in threaded view
|

Unique as special boxing type & hidden constructors

p.k.f.holzenspies@utwente.nl
Methinks a lot of the former performance considerations in Unique are out-dated (as per earlier discussion; direct use of unboxed ints etc.).


An upside of using an ADT for the types of uniques is that we don't actually need to reserve 8 bits for a Char (which is committing to neither the actual number of classes, nor the "nature" of real Chars in Haskell). Instead, we can make a bitmask dependent on the number of classes that we actually use and stick the tag on the least-significant side of the Unique, as opposed to the most-significant (as we do now).


We want to keep things working on 32-bits, but maybe a future of parallel builds is only for 64-bits. In this case, I would suggest that the 64-bit-case looks like this:


<thread_id_bits:8> <unique_id_bits:56-X> <tag_bits:X>


whereas the 32-bit case simply has


<unique_id_bits:32-X> <tag_bits:X>


Where X is dependent on the size of the UniqueClass-sum-type (to be introduced). This would be CPP-magic'd using ?WORD_SIZE_IN_BITS.


Ph.






________________________________
From: Simon Peyton Jones <simonpj at microsoft.com>
Sent: 20 August 2014 13:01
To: Holzenspies, P.K.F. (EWI); ghc-devs at haskell.org
Subject: RE: Unique as special boxing type & hidden constructors

Sounds like a good idea to me.   Would need to think about making sure that it all still worked, somehow, on 32 bit.

S

From: p.k.f.holzenspies at utwente.nl [mailto:p.k.f.holzenspies at utwente.nl]
Sent: 20 August 2014 11:31
To: Simon Peyton Jones; ghc-devs at haskell.org
Subject: RE: Unique as special boxing type & hidden constructors


Dear Simon, et al,



I seem to recall that the Unique(Supply) was an issue in parallelising GHC itself. There's a comment in the code (signed JSM) that there aren't any 64-bit bugs, if we have at least 32-bits for Ints and Chars fit in 8 characters. Then, there's bitmasks like 0x00FFFFFF to separate the "Int-part" from the "Char-part".



I was wondering; if we move Uniques to 64 bits, but use the top 16 (instead of the current 8) for *both* the tag (currently a Char, soon an sum-type) and the threadId of the supplying thread of a Unique, would that help?



Regards,

Philip









________________________________
From: Simon Peyton Jones <simonpj at microsoft.com<mailto:simonpj at microsoft.com>>
Sent: 18 August 2014 23:29
To: Holzenspies, P.K.F. (EWI); ghc-devs at haskell.org<mailto:ghc-devs at haskell.org>
Subject: RE: Unique as special boxing type & hidden constructors


1) There is a #ifdef define(__GLASGOW_HASKELL__), which confused me somewhat. Similar things occur elsewhere in the code. Isn't the assumption that GHC is being used? Is this old portability stuff that may be removed?

I think so, unless others yell to the contrary.


2) Uniques are produced from a Char and an Int. The function to build Uniques (mkUnique) is not exported, according to the comments, so as to see all characters used. Access to these different "classes" of Uniques is given through specialised mkXXXUnique functions. Does anyone have a problem with something like:



> data UniqueClass

>   = UniqDesugarer

>   | UniqAbsCFlattener

>   | UniqSimplStg

>   | UniqNativeCodeGen

>   ...

OK by me


3) Is there a reason for having functions implementing class-methods to be exported? In the case of Unique, there is pprUnique and:

> instance Outputable Unique where

>   ppr = pprUnique

Please don?t change this.  If you want to change how pretty-printing of uniques works, and want to find all the call sites of pprUnique, it?s FAR easier to grep for pprUnique than to search for  all calls of ppr, and work out which are at type Unique!

(In my view) it?s usually much better not to use type classes unless you actually need overloading.

Simon

From: p.k.f.holzenspies at utwente.nl<mailto:p.k.f.holzenspies at utwente.nl> [mailto:p.k.f.holzenspies at utwente.nl]
Sent: 18 August 2014 14:50
To: Simon Peyton Jones; ghc-devs at haskell.org<mailto:ghc-devs at haskell.org>
Subject: RE: Unique as special boxing type & hidden constructors


Dear Simon, et al,



Looking at Unique, there are a few more design choices that may be outdated, and since I'm polishing things now, anyway, I figured I could update it on more fronts.



1) There is a #ifdef define(__GLASGOW_HASKELL__), which confused me somewhat. Similar things occur elsewhere in the code. Isn't the assumption that GHC is being used? Is this old portability stuff that may be removed?



2) Uniques are produced from a Char and an Int. The function to build Uniques (mkUnique) is not exported, according to the comments, so as to see all characters used. Access to these different "classes" of Uniques is given through specialised mkXXXUnique functions. Does anyone have a problem with something like:



> data UniqueClass

>   = UniqDesugarer

>   | UniqAbsCFlattener

>   | UniqSimplStg

>   | UniqNativeCodeGen

>   ...



and a public (i.e. exported) function:



> mkUnique :: UniqueClass -> Int -> Unique



? The benefit of this would be to have more (to my taste) self-documenting code and a greater chance that documentation is updated (the list of "unique supply characters" in the comments is currently outdated).



3) Is there a reason for having functions implementing class-methods to be exported? In the case of Unique, there is pprUnique and:



> instance Outputable Unique where

>   ppr = pprUnique



Here pprUnique is exported and it is used in quite a few places where it's argument is unambiguously a Unique (so it's not to force the type) *and* "ppr" is used for all kinds of other types. I'm assuming this is an old choice making things marginally faster, but I would say cleaning up the API / namespace would now outweigh this margin.

?

I will also be adding Haddock-comments, so when this is done, a review would be most welcome (I'll also be doing some similar transformations to other long-since-untouched-code).



Regards,

Philip

















________________________________
Van: Simon Peyton Jones <simonpj at microsoft.com<mailto:simonpj at microsoft.com>>
Verzonden: maandag 18 augustus 2014 00:11
Aan: Holzenspies, P.K.F. (EWI); ghc-devs at haskell.org<mailto:ghc-devs at haskell.org>
Onderwerp: RE: Unique as special boxing type & hidden constructors

Re (1) I think this is a historical.  A newtype wrapping an Int should be fine.  I?d be ok with that change.

Re (2), I think your question is: why does module Unique export the data type Unique abstractly, rather than exporting both the data type and its constructor.  No deep reason here, but it guarantees that you can only *make* a unique from an Int by calling ?mkUniqueGrimily?, which signals clearly that something fishy is going on.  And rightly so!

Simon

From: ghc-devs [mailto:ghc-devs-bounces at haskell.org] On Behalf Of p.k.f.holzenspies at utwente.nl<mailto:p.k.f.holzenspies at utwente.nl>
Sent: 15 August 2014 11:53
To: ghc-devs at haskell.org<mailto:ghc-devs at haskell.org>
Subject: Unique as special boxing type & hidden constructors


Dear all,



I'm working with Alan to instantiate everything for Data.Data, so that we can do better SYB-traversals (which should also help newcomers significantly to get into the GHC code base). Alan's looking at the AST types, I'm looking at the basic types in the compiler.



Right now, I'm looking at Unique and two questions come up:



> data Unique = MkUnique FastInt



1) As someone already commented: Is there a specific reason (other than history) that this isn't simply a newtype around an Int? If we're boxing anyway, we may as well use the default Int boxing and newtype-coerce to the specific purpose of Unique, no?



2) As a general question for GHC hacking style; what is the reason for hiding the constructors in the first place?



I understand about abstraction and there are reasons for hiding, but there's a "public GHC API" and then there are all these modules that people can import at their own peril. Nothing is guaranteed about their consistency from version to version of GHC. I don't really see the point about hiding constructors (getting in the way of automatically deriving things) and then giving extra functions like (in the case of Unique):



> getKeyFastInt (MkUnique x) = x

> mkUniqueGrimily x = MkUnique (iUnbox x)



I would propose to just make Unique a newtype for an Int and making the constructor visible.

Regards,

Philip


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.haskell.org/pipermail/ghc-devs/attachments/20140820/6780e71d/attachment-0001.html>

Reply | Threaded
Open this post in threaded view
|

Unique as special boxing type & hidden constructors

Alexander Kjeldaas
On Wed, Aug 20, 2014 at 1:47 PM, <p.k.f.holzenspies at utwente.nl> wrote:

>  Methinks a lot of the former performance considerations in Unique are
> out-dated (as per earlier discussion; direct use of unboxed ints etc.).
>
>
>  An upside of using an ADT for the types of uniques is that we don't
> actually need to reserve 8 bits for a Char (which is committing to neither
> the actual number of classes, nor the "nature" of real Chars in Haskell).
> Instead, we can make a bitmask dependent on the number of classes that we
> actually use and stick the tag on the least-significant side of the Unique,
> as opposed to the most-significant (as we do now).
>
>
>  We want to keep things working on 32-bits, but maybe a future of
> parallel builds is only for 64-bits. In this case, I would suggest that the
> 64-bit-case looks like this:
>
>
>  <thread_id_bits:8> <unique_id_bits:56-X> <tag_bits:X>
>
>
>
Is the thread id deterministic between runs?  If not, please do not use
this layout.  I remember vaguely Unique being relevant to ghc not having
deterministic builds, my most wanted ghc feature:

https://ghc.haskell.org/trac/ghc/ticket/4012

Alexander



>  whereas the 32-bit case simply has
>
>
>  <unique_id_bits:32-X> <tag_bits:X>
>
>
>  Where X is dependent on the size of the UniqueClass-sum-type (to be
> introduced). This would be CPP-magic'd using ?WORD_SIZE_IN_BITS.
>
>
>  Ph.
>
>
>
>
>
>
>  ------------------------------
> *From:* Simon Peyton Jones <simonpj at microsoft.com>
> *Sent:* 20 August 2014 13:01
>
> *To:* Holzenspies, P.K.F. (EWI); ghc-devs at haskell.org
> *Subject:* RE: Unique as special boxing type & hidden constructors
>
>
> Sounds like a good idea to me.   Would need to think about making sure
> that it all still worked, somehow, on 32 bit.
>
>
>
> S
>
>
>
> *From:* p.k.f.holzenspies at utwente.nl [mailto:p.k.f.holzenspies at utwente.nl]
>
> *Sent:* 20 August 2014 11:31
> *To:* Simon Peyton Jones; ghc-devs at haskell.org
> *Subject:* RE: Unique as special boxing type & hidden constructors
>
>
>
> Dear Simon, et al,
>
>
>
> I seem to recall that the Unique(Supply) was an issue in parallelising GHC
> itself. There's a comment in the code (signed JSM) that there aren't any
> 64-bit bugs, if we have at least 32-bits for Ints and Chars fit in 8
> characters. Then, there's bitmasks like 0x00FFFFFF to separate the
> "Int-part" from the "Char-part".
>
>
>
> I was wondering; if we move Uniques to 64 bits, but use the top 16
> (instead of the current 8) for *both* the tag (currently a Char, soon an
> sum-type) and the threadId of the supplying thread of a Unique, would that
> help?
>
>
>
> Regards,
>
> Philip
>
>
>
>
>
>
>
>
>   ------------------------------
>
> *From:* Simon Peyton Jones <simonpj at microsoft.com>
> *Sent:* 18 August 2014 23:29
> *To:* Holzenspies, P.K.F. (EWI); ghc-devs at haskell.org
> *Subject:* RE: Unique as special boxing type & hidden constructors
>
>
>
> 1) There is a #ifdef define(__GLASGOW_HASKELL__), which confused me
> somewhat. Similar things occur elsewhere in the code. Isn't the assumption
> that GHC is being used? Is this old portability stuff that may be removed?
>
>
>
> I think so, unless others yell to the contrary.
>
>
>
> 2) Uniques are produced from a Char and an Int. The function to build
> Uniques (mkUnique) is not exported, according to the comments, so as to see
> all characters used. Access to these different "classes" of Uniques is
> given through specialised mkXXXUnique functions. Does anyone have a problem
> with something like:
>
>
>
> > data UniqueClass
>
> >   = UniqDesugarer
>
> >   | UniqAbsCFlattener
>
> >   | UniqSimplStg
>
> >   | UniqNativeCodeGen
>
> >   ...
>
>
>
> OK by me
>
>
>
> 3) Is there a reason for having functions implementing class-methods to be
> exported? In the case of Unique, there is pprUnique and:
>
> > instance Outputable Unique where
>
> >   ppr = pprUnique
>
>
>
> Please don?t change this.  If you want to change how pretty-printing of
> uniques works, and want to find all the call sites of pprUnique, it?s FAR
> easier to grep for pprUnique than to search for  all calls of ppr, and work
> out which are at type Unique!
>
>
>
> (In my view) it?s usually much better not to use type classes unless you
> actually need overloading.
>
>
>
> Simon
>
>
>
> *From:* p.k.f.holzenspies at utwente.nl [mailto:p.k.f.holzenspies at utwente.nl
> <p.k.f.holzenspies at utwente.nl>]
> *Sent:* 18 August 2014 14:50
> *To:* Simon Peyton Jones; ghc-devs at haskell.org
> *Subject:* RE: Unique as special boxing type & hidden constructors
>
>
>
> Dear Simon, et al,
>
>
>
> Looking at Unique, there are a few more design choices that may be
> outdated, and since I'm polishing things now, anyway, I figured I could
> update it on more fronts.
>
>
>
> 1) There is a #ifdef define(__GLASGOW_HASKELL__), which confused me
> somewhat. Similar things occur elsewhere in the code. Isn't the assumption
> that GHC is being used? Is this old portability stuff that may be removed?
>
>
>
> 2) Uniques are produced from a Char and an Int. The function to build
> Uniques (mkUnique) is not exported, according to the comments, so as to see
> all characters used. Access to these different "classes" of Uniques is
> given through specialised mkXXXUnique functions. Does anyone have a problem
> with something like:
>
>
>
> > data UniqueClass
>
> >   = UniqDesugarer
>
> >   | UniqAbsCFlattener
>
> >   | UniqSimplStg
>
> >   | UniqNativeCodeGen
>
> >   ...
>
>
>
> and a public (i.e. exported) function:
>
>
>
> > mkUnique :: UniqueClass -> Int -> Unique
>
>
>
> ? The benefit of this would be to have more (to my taste) self-documenting
> code and a greater chance that documentation is updated (the list of
> "unique supply characters" in the comments is currently outdated).
>
>
>
> 3) Is there a reason for having functions implementing class-methods to be
> exported? In the case of Unique, there is pprUnique and:
>
>
>
> > instance Outputable Unique where
>
> >   ppr = pprUnique
>
>
>
> Here pprUnique is exported and it is used in quite a few places where it's
> argument is unambiguously a Unique (so it's not to force the type) *and*
> "ppr" is used for all kinds of other types. I'm assuming this is an old
> choice making things marginally faster, but I would say cleaning up the API
> / namespace would now outweigh this margin.
>
> ?
>
> I will also be adding Haddock-comments, so when this is done, a review
> would be most welcome (I'll also be doing some similar transformations to
> other long-since-untouched-code).
>
>
>
> Regards,
>
> Philip
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>   ------------------------------
>
> *Van:* Simon Peyton Jones <simonpj at microsoft.com>
> *Verzonden:* maandag 18 augustus 2014 00:11
> *Aan:* Holzenspies, P.K.F. (EWI); ghc-devs at haskell.org
> *Onderwerp:* RE: Unique as special boxing type & hidden constructors
>
>
>
> Re (1) I think this is a historical.  A newtype wrapping an Int should be
> fine.  I?d be ok with that change.
>
>
>
> Re (2), I think your question is: why does module Unique export the data
> type Unique abstractly, rather than exporting both the data type and its
> constructor.  No deep reason here, but it guarantees that you can only *
> *make** a unique from an Int by calling ?mkUniqueGrimily?, which signals
> clearly that something fishy is going on.  And rightly so!
>
>
>
> Simon
>
>
>
> *From:* ghc-devs [mailto:ghc-devs-bounces at haskell.org
> <ghc-devs-bounces at haskell.org>] *On Behalf Of *
> p.k.f.holzenspies at utwente.nl
> *Sent:* 15 August 2014 11:53
> *To:* ghc-devs at haskell.org
> *Subject:* Unique as special boxing type & hidden constructors
>
>
>
> Dear all,
>
>
>
> I'm working with Alan to instantiate everything for Data.Data, so that we
> can do better SYB-traversals (which should also help newcomers
> significantly to get into the GHC code base). Alan's looking at the AST
> types, I'm looking at the basic types in the compiler.
>
>
>
> Right now, I'm looking at Unique and two questions come up:
>
>
>
> > data Unique = MkUnique FastInt
>
>
>
> 1) As someone already commented: Is there a specific reason (other than
> history) that this isn't simply a newtype around an Int? If we're boxing
> anyway, we may as well use the default Int boxing and newtype-coerce to the
> specific purpose of Unique, no?
>
>
>
> 2) As a general question for GHC hacking style; what is the reason for
> hiding the constructors in the first place?
>
>
>
> I understand about abstraction and there are reasons for hiding, but
> there's a "public GHC API" and then there are all these modules that people
> can import at their own peril. Nothing is guaranteed about their
> consistency from version to version of GHC. I don't really see the point
> about hiding constructors (getting in the way of automatically deriving
> things) and then giving extra functions like (in the case of Unique):
>
>
>
> > getKeyFastInt (MkUnique x) = x
>
> > mkUniqueGrimily x = MkUnique (iUnbox x)
>
>
>
> I would propose to just make Unique a newtype for an Int and making the
> constructor visible.
>
> Regards,
>
> Philip
>
>
>
> _______________________________________________
> ghc-devs mailing list
> ghc-devs at haskell.org
> http://www.haskell.org/mailman/listinfo/ghc-devs
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.haskell.org/pipermail/ghc-devs/attachments/20140820/41f321d9/attachment-0001.html>

Reply | Threaded
Open this post in threaded view
|

Unique as special boxing type & hidden constructors

p.k.f.holzenspies@utwente.nl
On Wed, Aug 20, 2014 at 1:47 PM, <p.k.f.holzenspies at utwente.nl<mailto:p.k.f.holzenspies at utwente.nl>> wrote:


<thread_id_bits:8> <unique_id_bits:56-X> <tag_bits:X>


Is the thread id deterministic between runs?  If not, please do not use this layout.  I remember vaguely Unique being relevant to ghc not having deterministic builds, my most wanted ghc feature:

https://ghc.haskell.org/trac/ghc/ticket/4012


I think this depends on the policy GHC *will* have (there is not parallel build atm) wrt. the forking of threads. An actual Control.Concurrent.ThreadId might be as large as 64 bits, so, of course, we won't be using that, but rather the sequence number in which the UniqueSupply was "split off" for a new thread. In other words, if the decision to fork threads is deterministic, so are the Uniques with this layout.

Mind you, I imagine a parallel GHC would still have at most one thread working on a single module. I don't know too much about what makes it into the interface file of a module (I can't imagine the exact Uniques end up there, because they would overlap with other modules - with per-module compilation - and conflict that way).

Can you comment on how (the layout of) Uniques relate to #4012 in a little more detail? It seems that if the Uniques that somehow end up in the interface files could simply be stripped of the thread id, in which case, the problem reduces to the current one.

Ph.




-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.haskell.org/pipermail/ghc-devs/attachments/20140820/09b26544/attachment.html>

Reply | Threaded
Open this post in threaded view
|

Unique as special boxing type & hidden constructors

Alexander Kjeldaas
On Wed, Aug 20, 2014 at 3:07 PM, <p.k.f.holzenspies at utwente.nl> wrote:

>   On Wed, Aug 20, 2014 at 1:47 PM, <p.k.f.holzenspies at utwente.nl> wrote:
>
>>
>>         <thread_id_bits:8> <unique_id_bits:56-X> <tag_bits:X>
>>
>
>>
>      Is the thread id deterministic between runs?  If not, please do not
> use this layout.  I remember vaguely Unique being relevant to ghc not
> having deterministic builds, my most wanted ghc feature:
>
>      https://ghc.haskell.org/trac/ghc/ticket/4012
>
>
>  I think this depends on the policy GHC *will* have (there is not
> parallel build atm) wrt. the forking of threads. An actual
> Control.Concurrent.ThreadId might be as large as 64 bits, so, of course, we
> won't be using that, but rather the sequence number in which the
> UniqueSupply was "split off" for a new thread. In other words, if the
> decision to fork threads is deterministic, so are the Uniques with this
> layout.
>
>  Mind you, I imagine a parallel GHC would still have at most one thread
> working on a single module. I don't know too much about what makes it into
> the interface file of a module (I can't imagine the exact Uniques end up
> there, because they would overlap with other modules - with per-module
> compilation - and conflict that way).
>
>  Can you comment on how (the layout of) Uniques relate to #4012 in a
> little more detail? It seems that if the Uniques that somehow end up in the
> interface files could simply be stripped of the thread id, in which case,
> the problem reduces to the current one.
>
>
I frankly don't know.  I just think it's better to keep ThreadId out of
data that can bleed into symbols and what not.

As you can see, the thread id is just a counter, and as forkIO in a
threaded runtime will be racy between threads, they aren't deterministic.

http://stackoverflow.com/questions/24995262/how-can-i-build-a-threadid-given-that-i-know-the-actual-number


Alexander
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.haskell.org/pipermail/ghc-devs/attachments/20140820/c99ec47e/attachment-0001.html>

Reply | Threaded
Open this post in threaded view
|

Unique as special boxing type & hidden constructors

p.k.f.holzenspies@utwente.nl
Dear Max, et al,


Here's hoping either you are still on the mailing list, or the address I found on your website (which says you're a Ph.D. student, so it's starting to smell) is still operational.


I'm working on redoing some Unique-stuff in GHC. Mostly, the code uses Unique's API in a well-behaved fashion. The only awkward bit I found is in BinIface.getSymtabName, which git blames you for ;)


I just wanted to ask: Why does this functions do all the bit-masking and shifting stuff directly and with different masks than anything in Unique? Is there a reason why this doesn't use unpkUnique? The comments in Unique state that mkUnique is NOT EXPORTED (the caps are in the comments, I'm not shouting), but they are, it seems, specifically for BinIface. I would like to get rid of this, but dare not hack away in the dark.


Regards,

Philip




________________________________
From: Alexander Kjeldaas <alexander.kjeldaas at gmail.com>
Sent: 20 August 2014 15:48
To: Holzenspies, P.K.F. (EWI)
Cc: Simon Peyton Jones; ghc-devs
Subject: Re: Unique as special boxing type & hidden constructors




On Wed, Aug 20, 2014 at 3:07 PM, <p.k.f.holzenspies at utwente.nl<mailto:p.k.f.holzenspies at utwente.nl>> wrote:

On Wed, Aug 20, 2014 at 1:47 PM, <p.k.f.holzenspies at utwente.nl<mailto:p.k.f.holzenspies at utwente.nl>> wrote:


<thread_id_bits:8> <unique_id_bits:56-X> <tag_bits:X>


Is the thread id deterministic between runs?  If not, please do not use this layout.  I remember vaguely Unique being relevant to ghc not having deterministic builds, my most wanted ghc feature:

https://ghc.haskell.org/trac/ghc/ticket/4012


I think this depends on the policy GHC *will* have (there is not parallel build atm) wrt. the forking of threads. An actual Control.Concurrent.ThreadId might be as large as 64 bits, so, of course, we won't be using that, but rather the sequence number in which the UniqueSupply was "split off" for a new thread. In other words, if the decision to fork threads is deterministic, so are the Uniques with this layout.

Mind you, I imagine a parallel GHC would still have at most one thread working on a single module. I don't know too much about what makes it into the interface file of a module (I can't imagine the exact Uniques end up there, because they would overlap with other modules - with per-module compilation - and conflict that way).

Can you comment on how (the layout of) Uniques relate to #4012 in a little more detail? It seems that if the Uniques that somehow end up in the interface files could simply be stripped of the thread id, in which case, the problem reduces to the current one.


I frankly don't know.  I just think it's better to keep ThreadId out of data that can bleed into symbols and what not.

As you can see, the thread id is just a counter, and as forkIO in a threaded runtime will be racy between threads, they aren't deterministic.

http://stackoverflow.com/questions/24995262/how-can-i-build-a-threadid-given-that-i-know-the-actual-number


Alexander

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.haskell.org/pipermail/ghc-devs/attachments/20140820/a6b905c6/attachment.html>

Reply | Threaded
Open this post in threaded view
|

Unique as special boxing type & hidden constructors

Simon Marlow-7
In reply to this post by Edward Z. Yang
FastInt = Int#, so newtype doesn't work here.

Cheers,
Simon

On 15/08/2014 14:01, Edward Z. Yang wrote:

> The definition dates back to 1996, so it seems plausible that
> newtype is the way to go now.
>
> Edward
>
> Excerpts from p.k.f.holzenspies's message of 2014-08-15 11:52:47 +0100:
>> Dear all,
>>
>>
>> I'm working with Alan to instantiate everything for Data.Data, so that we can do better SYB-traversals (which should also help newcomers significantly to get into the GHC code base). Alan's looking at the AST types, I'm looking at the basic types in the compiler.
>>
>> Right now, I'm looking at Unique and two questions come up:
>>
>>> data Unique = MkUnique FastInt
>>
>>
>> 1) As someone already commented: Is there a specific reason (other than history) that this isn't simply a newtype around an Int? If we're boxing anyway, we may as well use the default Int boxing and newtype-coerce to the specific purpose of Unique, no?
>>
>>
>> 2) As a general question for GHC hacking style; what is the reason for hiding the constructors in the first place?
>>
>> I understand about abstraction and there are reasons for hiding, but there's a "public GHC API" and then there are all these modules that people can import at their own peril. Nothing is guaranteed about their consistency from version to version of GHC. I don't really see the point about hiding constructors (getting in the way of automatically deriving things) and then giving extra functions like (in the case of Unique):
>>
>>> getKeyFastInt (MkUnique x) = x
>>
>>> mkUniqueGrimily x = MkUnique (iUnbox x)
>>
>>
>> I would propose to just make Unique a newtype for an Int and making the constructor visible.
>>
>>
>> Regards,
>>
>> Philip
> _______________________________________________
> ghc-devs mailing list
> ghc-devs at haskell.org
> http://www.haskell.org/mailman/listinfo/ghc-devs
>

Reply | Threaded
Open this post in threaded view
|

Unique as special boxing type & hidden constructors

Edward Z. Yang
Newtype of Int, to be clear.

Edward

Excerpts from Simon Marlow's message of 2014-09-04 11:49:39 +0200:

> FastInt = Int#, so newtype doesn't work here.
>
> Cheers,
> Simon
>
> On 15/08/2014 14:01, Edward Z. Yang wrote:
> > The definition dates back to 1996, so it seems plausible that
> > newtype is the way to go now.
> >
> > Edward
> >
> > Excerpts from p.k.f.holzenspies's message of 2014-08-15 11:52:47 +0100:
> >> Dear all,
> >>
> >>
> >> I'm working with Alan to instantiate everything for Data.Data, so that we can do better SYB-traversals (which should also help newcomers significantly to get into the GHC code base). Alan's looking at the AST types, I'm looking at the basic types in the compiler.
> >>
> >> Right now, I'm looking at Unique and two questions come up:
> >>
> >>> data Unique = MkUnique FastInt
> >>
> >>
> >> 1) As someone already commented: Is there a specific reason (other than history) that this isn't simply a newtype around an Int? If we're boxing anyway, we may as well use the default Int boxing and newtype-coerce to the specific purpose of Unique, no?
> >>
> >>
> >> 2) As a general question for GHC hacking style; what is the reason for hiding the constructors in the first place?
> >>
> >> I understand about abstraction and there are reasons for hiding, but there's a "public GHC API" and then there are all these modules that people can import at their own peril. Nothing is guaranteed about their consistency from version to version of GHC. I don't really see the point about hiding constructors (getting in the way of automatically deriving things) and then giving extra functions like (in the case of Unique):
> >>
> >>> getKeyFastInt (MkUnique x) = x
> >>
> >>> mkUniqueGrimily x = MkUnique (iUnbox x)
> >>
> >>
> >> I would propose to just make Unique a newtype for an Int and making the constructor visible.
> >>
> >>
> >> Regards,
> >>
> >> Philip
> > _______________________________________________
> > ghc-devs mailing list
> > ghc-devs at haskell.org
> > http://www.haskell.org/mailman/listinfo/ghc-devs
> >

Reply | Threaded
Open this post in threaded view
|

Unique as special boxing type & hidden constructors

p.k.f.holzenspies@utwente.nl
In reply to this post by Simon Marlow-7
Dear Simon,

The point is to have

newtype Unique = Unique Int

where we use the boxing of Int, instead of creating our own boxing. Actually, it seems useful to move to

newtype Unique = Unique Word

(see other discussions about unnecessary signedness).

I've been working on this (although only as a side-project, so progress is very slow) and I've discovered a lot of API-out-of-sync-ness; there are comments stating we don't export mkUnique, so that we can keep track of all the Chars we use. Unfortunately, we *do* export mkUnique from Unique and we do *not* have consistent use of Chars everywhere. I'm working to replace the Char-mechanism with a (rather straightforward) sum-type UniqueDomain. This should also help get a more consistent treatment of serialisation (the one in the module Unique is *slightly* different from the one in BinIface).

I'm still not quite sure how to do the performance tests on the actual compilation (i.e. runtime of GHC itself). If anything, moving Uniques to a higher abstraction (coerced boxed values, instead of manually boxed stuff) is actually a good litmus test of how far GHC's optimisations have come since '96 ;)

If you have any more input, especially on performance stuff (what would be the worst acceptable performance hit and measured on what, for example), it would be *very* welcome!

Regards,
Philip



________________________________________
From: Simon Marlow <marlowsd at gmail.com>
Sent: 04 September 2014 11:49
To: Edward Z. Yang; Holzenspies, P.K.F. (EWI)
Cc: ghc-devs
Subject: Re: Unique as special boxing type & hidden constructors

FastInt = Int#, so newtype doesn't work here.

Cheers,
Simon

On 15/08/2014 14:01, Edward Z. Yang wrote:

> The definition dates back to 1996, so it seems plausible that
> newtype is the way to go now.
>
> Edward
>
> Excerpts from p.k.f.holzenspies's message of 2014-08-15 11:52:47 +0100:
>> Dear all,
>>
>>
>> I'm working with Alan to instantiate everything for Data.Data, so that we can do better SYB-traversals (which should also help newcomers significantly to get into the GHC code base). Alan's looking at the AST types, I'm looking at the basic types in the compiler.
>>
>> Right now, I'm looking at Unique and two questions come up:
>>
>>> data Unique = MkUnique FastInt
>>
>>
>> 1) As someone already commented: Is there a specific reason (other than history) that this isn't simply a newtype around an Int? If we're boxing anyway, we may as well use the default Int boxing and newtype-coerce to the specific purpose of Unique, no?
>>
>>
>> 2) As a general question for GHC hacking style; what is the reason for hiding the constructors in the first place?
>>
>> I understand about abstraction and there are reasons for hiding, but there's a "public GHC API" and then there are all these modules that people can import at their own peril. Nothing is guaranteed about their consistency from version to version of GHC. I don't really see the point about hiding constructors (getting in the way of automatically deriving things) and then giving extra functions like (in the case of Unique):
>>
>>> getKeyFastInt (MkUnique x) = x
>>
>>> mkUniqueGrimily x = MkUnique (iUnbox x)
>>
>>
>> I would propose to just make Unique a newtype for an Int and making the constructor visible.
>>
>>
>> Regards,
>>
>> Philip
> _______________________________________________
> ghc-devs mailing list
> ghc-devs at haskell.org
> http://www.haskell.org/mailman/listinfo/ghc-devs
>

Reply | Threaded
Open this post in threaded view
|

Unique as special boxing type & hidden constructors

Simon Marlow-7
On 04/09/2014 13:25, p.k.f.holzenspies at utwente.nl wrote:
> Dear Simon,
>
> The point is to have
>
> newtype Unique = Unique Int

Yes, I misunderstood, thanks!

> where we use the boxing of Int, instead of creating our own boxing. Actually, it seems useful to move to
>
> newtype Unique = Unique Word
>
> (see other discussions about unnecessary signedness).
>
> I've been working on this (although only as a side-project, so progress is very slow) and I've discovered a lot of API-out-of-sync-ness; there are comments stating we don't export mkUnique, so that we can keep track of all the Chars we use. Unfortunately, we *do* export mkUnique from Unique and we do *not* have consistent use of Chars everywhere. I'm working to replace the Char-mechanism with a (rather straightforward) sum-type UniqueDomain. This should also help get a more consistent treatment of serialisation (the one in the module Unique is *slightly* different from the one in BinIface).
>
> I'm still not quite sure how to do the performance tests on the actual compilation (i.e. runtime of GHC itself). If anything, moving Uniques to a higher abstraction (coerced boxed values, instead of manually boxed stuff) is actually a good litmus test of how far GHC's optimisations have come since '96 ;)
>
> If you have any more input, especially on performance stuff (what would be the worst acceptable performance hit and measured on what, for example), it would be *very* welcome!

There are compiler performance tests in testsuite/tests/perf/compiler.
They will only fail if the performance gets worse by 10% or so
(depending on the test), so instead of checking for failure just run the
tests manually and compare the allocation values before and after your
change (remember to save a copy of the original compiler binary so that
you can do comparisons).

Another way to test performance is to compile a large module, e.g.
nofib/spectral/simple/Main.hs is a good one.

Cheers,
Simon


> Regards,
> Philip
>
>
>
> ________________________________________
> From: Simon Marlow <marlowsd at gmail.com>
> Sent: 04 September 2014 11:49
> To: Edward Z. Yang; Holzenspies, P.K.F. (EWI)
> Cc: ghc-devs
> Subject: Re: Unique as special boxing type & hidden constructors
>
> FastInt = Int#, so newtype doesn't work here.
>
> Cheers,
> Simon
>
> On 15/08/2014 14:01, Edward Z. Yang wrote:
>> The definition dates back to 1996, so it seems plausible that
>> newtype is the way to go now.
>>
>> Edward
>>
>> Excerpts from p.k.f.holzenspies's message of 2014-08-15 11:52:47 +0100:
>>> Dear all,
>>>
>>>
>>> I'm working with Alan to instantiate everything for Data.Data, so that we can do better SYB-traversals (which should also help newcomers significantly to get into the GHC code base). Alan's looking at the AST types, I'm looking at the basic types in the compiler.
>>>
>>> Right now, I'm looking at Unique and two questions come up:
>>>
>>>> data Unique = MkUnique FastInt
>>>
>>>
>>> 1) As someone already commented: Is there a specific reason (other than history) that this isn't simply a newtype around an Int? If we're boxing anyway, we may as well use the default Int boxing and newtype-coerce to the specific purpose of Unique, no?
>>>
>>>
>>> 2) As a general question for GHC hacking style; what is the reason for hiding the constructors in the first place?
>>>
>>> I understand about abstraction and there are reasons for hiding, but there's a "public GHC API" and then there are all these modules that people can import at their own peril. Nothing is guaranteed about their consistency from version to version of GHC. I don't really see the point about hiding constructors (getting in the way of automatically deriving things) and then giving extra functions like (in the case of Unique):
>>>
>>>> getKeyFastInt (MkUnique x) = x
>>>
>>>> mkUniqueGrimily x = MkUnique (iUnbox x)
>>>
>>>
>>> I would propose to just make Unique a newtype for an Int and making the constructor visible.
>>>
>>>
>>> Regards,
>>>
>>> Philip
>> _______________________________________________
>> ghc-devs mailing list
>> ghc-devs at haskell.org
>> http://www.haskell.org/mailman/listinfo/ghc-devs
>>
>