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

proposal: add standard library package for sizes #33459

Closed
RXminuS opened this issue Aug 4, 2019 · 20 comments
Closed

proposal: add standard library package for sizes #33459

RXminuS opened this issue Aug 4, 2019 · 20 comments

Comments

@RXminuS
Copy link

RXminuS commented Aug 4, 2019

This is a duplicate of #19375 that has been frozen.

But given that Go2 is now in the works I thought it's good to reopen the discussion of adding a sizes unit just how working with time is amazing.

The main motivation for us is that there is a lot of places in code where you see 2 * 1024 * 1024 which is not very readable or clear. Especially since some Gophers do 2 * 1000 * 1000 or just write 2000000.

It would be much nicer if you could do Size("2MiB") or LT("1KB"). There is a couple of Go packages that do this already (I think Docker has one) but it seems to me that this is such a core expression (just like time) and so confusing to new programmers that I think it might be a good idea to include within the Go2 core.

@gopherbot gopherbot added this to the Proposal milestone Aug 4, 2019
@nsajko
Copy link
Contributor

nsajko commented Aug 4, 2019

Iota is expressive enough to solve your problem easily: https://play.golang.org/p/fyiUtE1qD70

@ianlancetaylor ianlancetaylor changed the title Proposal: go2 bytes size units proposal: add standard library package for sizes Aug 5, 2019
@ianlancetaylor ianlancetaylor added the v2 A language change or incompatible library change label Aug 5, 2019
@ianlancetaylor
Copy link
Contributor

I think you are suggesting that we add a new standard library package for this, and I've retitled the issue accordingly.

Is there an existing package outside of the standard library that does what you want?

In general we would need to work out a complete API. But I suppose that first we should decide whether we need it. https://golang.org/doc/faq#x_in_std.

@RXminuS
Copy link
Author

RXminuS commented Aug 5, 2019

@nsajko that's sort of beside the point I was hoping to convey in that this is something that is currently very fragmented. Usually with go code, if you look at someone else's it feels like you wrote it, but this particular point is always different.

Adding a standard package around it would unify the code in an idiomatic way. Especially since it can be such a lightweight implementation.

Here's one package we've used before https://github.com/docker/go-units

@smasher164
Copy link
Member

Is there existing code in the standard library that can use such a package? That may help support your point on unification.

Otherwise, I don’t see why this has to live in the standard library. An x/ repo might be a better place for it, if external code doesn’t want to use packages like the one you mentioned.

@RXminuS
Copy link
Author

RXminuS commented Aug 13, 2019

That might be a better place, didn't realise that's where Time is as well

@RXminuS
Copy link
Author

RXminuS commented Aug 13, 2019

I'll have a look through the core because it must be used somewhere though

@tv42
Copy link

tv42 commented Aug 13, 2019

Size("2MiB") would have to return an error, though. Plus if it's a function call, you can't use it with const.

@ianlancetaylor ianlancetaylor removed the v2 A language change or incompatible library change label Aug 27, 2019
@ianlancetaylor
Copy link
Contributor

One useful way to handle constants like 2 * 1024 * 1024 is to use a shift. 2 << 10 is a number of kilobytes, 2 << 20 is a number of megabytes, etc.

@ianlancetaylor
Copy link
Contributor

Given that there is an existing package https://github.com/docker/go-units, what is the advantage of putting the package in the standard library? Bear in mind the discussion at https://golang.org/doc/faq#x_in_std.

@RXminuS
Copy link
Author

RXminuS commented Sep 9, 2019

The reason for standardizing is exactly Because there’s lots of “good ways” to express sizes. However if code is to be optimized for readability then 2 << 10 is not at all expressive (especially for new programmers). Even though it’s the one I use I also feel it’s a relic left over from Assembly programming. The problem gets amplified since depending on the programmer they might write 2* 1024, 2048, 2000 Or 2* 1000...non of which expresses the semantic meaning and in a few cases can even lead to weird bugs only detectable with fuzzy tests because of incompatibility between them.

@ianlancetaylor
Copy link
Contributor

While 2 << 10 may not be expressive, it is a constant expression. sizes.Size("2MiB") is not; it will require calling a function that parses a string. Even the slightest concern for efficiency requires using something more like 2 << 10.

@RXminuS
Copy link
Author

RXminuS commented Sep 10, 2019

But I'm sure we can optimise that in the compiler? Also, I thought Go's tradeoffs leaned towards readability, not 0 cost abstractions? We have Rust for that right?

@RXminuS
Copy link
Author

RXminuS commented Sep 10, 2019

Just to be clear I'm not saying it has to be a function call, I'm just saying that the developer experience currently is broken

@RXminuS
Copy link
Author

RXminuS commented Sep 10, 2019

We could always have a set of constants called MiB, GiB, GB etc. And then you just multiply them. Then the expression would be Bytes(3) * MiB

@ianlancetaylor
Copy link
Contributor

I'm just saying that the developer experience currently is broken

I think that statement is a bit strong.

We could always have a set of constants called MiB, GiB, GB etc. And then you just multiply them. Then the expression would be Bytes(3) * MiB

That seems like an entirely different proposal. Want to write that one out in some more detail?

(Personally I get confused between GiB and GB, but that's just me.)

@RXminuS
Copy link
Author

RXminuS commented Sep 12, 2019

I can write a proposal but tbh, I don't feel like I'm good enough to do so. Sure I have an opinion and I have seen this become a problem within a few teams which is why I filed this more as a bug report.

But I just read the proposal for generics and it's so considerate of all the aspects of Go's design and compiler architecture...I don't have enough of an understanding or empathy with those to make something like that.

With regard to that the experience is broken, yeah that might be a bit strong (apologies) but it's certainly a point of friction and ambiguity at times. I can't make a balanced decision on what the best way to solve it is but would love to be part of the process.

I think just having constants defined for MiB, GiB, TiB, and B might be enough since they actually capture the semantic meaning of a value in its purest form...the amount of actual bits. If you were to allocate an array of that size you know exactly how many characters or u32s you can fit into it. I don't think that the features of Time for comparison and arithmetic are as important since this eperstions are rare and are performed in % 10 not % 60 & % 24.

But if feel that if there is an idiomatic and standard way of expressing byte sizes that would be enough of a push (in general Go developers strive for idiomatic fluency).

But also completely understand if it is thought of as too trivial to bloat the language with.

@rsc
Copy link
Contributor

rsc commented Sep 12, 2019

It seems like not much has changed since #19375 (declined at #19375 (comment)).

Specifically:

  • Adding a new type would conflict with all APIs that take int64 for byte sizes (like Seek, result of Read, etc.)
  • Adding untyped constants seems maybe OK but verbose, and not byte-specific.
  • When there's a package name involved, you'd have something like 3 * sizes.K which doesn't seem particularly concise.
  • Worse, I can't tell at a glance whether 3 * sizes.K is 3 * 1024 (3<<10) or 3 * 1000.

I can understand why in the abstract it would be nice to write '3 MB' instead of 3_000_000 or 3e6 or 3<<20. But if you want to make that happen we need a clear proposal that doesn't add more complexity than it removes.

The "optimized away at compile time" function call seems to have been dismissed already, but I will just note that in addition to not being something we've ever done with Go, it would eliminate any possibility of compile-time errors on misspellings, like "3NB" instead of "3MB".

There does not seem to be a clear path forward here. Any takers on laying one out?

@RXminuS
Copy link
Author

RXminuS commented Sep 13, 2019

Those are very fair observations, thank you.

Maybe it's something that can be added/solved by go fmt? I think as long as you settle around a 3<<10 rather than 3×1024 or 3*1000 being sent in as size params that might make it more consistent? A gentle nudge might be enough...

@ianlancetaylor
Copy link
Contributor

gofmt shouldn't normally rewrite people's code other than adjusting the formatting.

@rsc
Copy link
Contributor

rsc commented Sep 25, 2019

Given the lack of a clear path forward here, this seems like a likely decline.

Leaving open for a week for final comments.

@rsc rsc closed this as completed Oct 2, 2019
@golang golang locked and limited conversation to collaborators Oct 1, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

7 participants