Skip to content

[std] mem.readInt is confusing and hostile to optimization #638

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

Closed
thejoshwolfe opened this issue Nov 30, 2017 · 4 comments
Closed

[std] mem.readInt is confusing and hostile to optimization #638

thejoshwolfe opened this issue Nov 30, 2017 · 4 comments
Labels
breaking Implementing this issue could cause existing code to no longer compile or have different behavior. standard library This issue involves writing Zig code for the standard library.
Milestone

Comments

@thejoshwolfe
Copy link
Contributor

The loop bounds are determined from the runtime-known bytes.len instead of the comptime-known @sizeOf(T). This prevents/discourages the compiler from unrolling the loop. For native endian reads, mem.readInt should optimize to a single (unaligned) load instruction, but this seems unlikely with the current API.

Why is there an API for reading an int from too few bytes? When would you want that? This was surprising to me when I read what readInt would do.

Proposal: Move the current API to something that indicates how weird it is, like mem.readPartialInt. I've already pushed optimizable implementations of readIntLE and readIntBE which you can use if you know at comptime which one you want. Add a mem.readIntWithEndian that takes a runtime-known endian and calls one of readIntLE or readIntBE. The long name is to discourage you from calling it unless you really only know the endian at runtime, which seems like a rare usecase.

@thejoshwolfe thejoshwolfe added the breaking Implementing this issue could cause existing code to no longer compile or have different behavior. label Nov 30, 2017
@andrewrk andrewrk added this to the 0.2.0 milestone Nov 30, 2017
@andrewrk
Copy link
Member

Why is there an API for reading an int from too few bytes?

The docs are:

/// Reads an integer from memory with size equal to bytes.len.
/// T specifies the return type, which must be large enough to store
/// the result.

Where are you getting this idea from?

@thejoshwolfe
Copy link
Contributor Author

Why is there an API for reading an int from too few bytes?

https://github.com/zig-lang/zig/blob/5786df933d37d52d57fef9c28acb9c2c23128d31/std/mem.zig#L387-L396

@andrewrk
Copy link
Member

Reads an integer from memory with size equal to bytes.len.

The above example has buf.len == 4, therefore it reads a 32-bit integer.

/// T specifies the return type, which must be large enough to store
/// the result.

The above example passes u64 as T, and therefore the return type is u64.

64 bits >= 32 bits. Where does "too few bytes" come in?

@andrewrk
Copy link
Member

https://github.com/zig-lang/zig/blob/5786df933d37d52d57fef9c28acb9c2c23128d31/std/debug.zig#L306

Here's the usage. Feel free to refactor this as you see fit if you come up with a better API.

@andrewrk andrewrk added the standard library This issue involves writing Zig code for the standard library. label Dec 6, 2017
@andrewrk andrewrk modified the milestones: 0.2.0, 0.3.0 Dec 8, 2017
@andrewrk andrewrk modified the milestones: 0.3.0, 0.4.0 Feb 28, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking Implementing this issue could cause existing code to no longer compile or have different behavior. standard library This issue involves writing Zig code for the standard library.
Projects
None yet
Development

No branches or pull requests

2 participants