After some consideration, I believe that using getOrPutAssumeCapacity cannot solve the problem. The reasons are as follows: getOrPut can indeed claim resources, but we need to note, is it only claiming memory resources? No, in fact the crucial resource it claims is an undefined key-value pair in the hashmap. The problem that ultimately arises is not a memory leak, but that the undefined key-value pair it claimed is left unhandled.
When we use getOrPutAssumeCapacity, it indeed doesn’t allocate memory, but does that mean it doesn’t claim an undefined key-value pair resource? No, it still claims it.
So, if it isn’t properly handled with errdefer if (!gop.found_existing) state.string_table.removeByPtr(gop.key_ptr), it still seems problematic to me.
Allocating all memory in advance and then asserting with errdefer comptime unreachable that no error will occur seems effective, but it only works for memory allocation errors. A completely plausible scenario is: if !gop.result, information is fetched from Io to fill the value. At this point, it is entirely possible for an error to be reported, and it cannot be avoided by pre-allocating memory or similar strategies.
This isn’t plausible because you aren’t allowed to modify the key, !gop.found_existing signals that you should initialize the value, the key has to remain unchanged.
If key exists this function cannot fail. If there is an existing item with key, then the result’s Entry pointers point to it, and found_existing is true. Otherwise, puts a new item with undefined value, and the Entry pointers point to it. Caller should then initialize the value (but not the key).
To be able to call getOrPut you already need to have the final version of the key.
I sincerely apologize; this was a typo caused by translation software. I have already changed key to value.
Edit: My original intention was to express “the value corresponding to this key”. In any case, I deleted the “of the hashmap key” that could cause confusion.
The fixed version first: reserves capacity for the string, then reserves capacity for the key/value pair, then asserts no more errors are possible, and then, only then, does the operation which can result in the return of uninitialized memory.
Nothing after that is allowed to fail, so there is no risk of the returned value exposing uninitialized memory. I suppose the code could fail to add the value, but that’s a stretch. Reserved capacity is itself uninitialized, but the data structure itself handles that safely.
It’s a bit of a tradeoff, because you’re reserving some amount of extra memory whether or not it’s needed. I don’t see that as a problem.
So something like this:
Cannot run below errdefer comptime unreachable, because anything below that which can return an error will not compile.
Useful for this specific scenario, but it’s only a locally optimal solution. Because the only possible errors in this scenario happen to only be related to memory allocation, this solution can work.
Consider the scenario where, during !gop.found_existing , the value is filled through Io. Io-related errors cannot be avoided by pre-allocating memory.
errdefer comptime unreachable; always works, because it’s either true or it isn’t, and if it isn’t, the code won’t compile.
In any case, I had thought you were talking about the example from the blog. Instead, you appear to be talking about a different example, which works in some other fashion, although you didn’t provide that example, or make it clear that you were doing so.
But of course in that circumstance, anything at all could be true. So I don’t have anything useful to reply to it.
Yes, this assertion can ensure that no error is thrown when populating gop, so it will cause a compilation failure.
However, there are real scenarios where there is a clear need to throw an error when retrieving values for undefined key-value pairs while populating gop. Since this error comes from I/O, we cannot avoid it in advance by pre-allocating memory to achieve the functionality we need.
When we use errdefer comptime unreachable to assert that no errors can occur afterwards, we simultaneously forbid needs that actually exist.
One example is using a hashmap as an in-memory lazy-loaded key-value store for remote data, where there is a need to get pointers to keys and values. It can be asserted that the requested data exists remotely, but may not necessarily exist locally. getOrPut directly obtains pointers to the key and value if the data is found locally. If it is not found, it fetches the value from the remote data, fills in the value, and then returns pointers to the key and value.
Without using getOrPut, you would need to take a lot of detours: getKeyPtr - putNoClobber - getKeyPtr/getPtr
When you fetch from a remote the overhead of doing
is likely so minuscule in the grand scheme of things, that I would just take it.
As an aside: Putting a network request into such a call, which I normally expect to be fast could get bad really fast. So I would probably create two versions tryGet/getCached and getRemote or something where one just asks the cache and the other does the expensive call.
I get this point, the additional overhead of API call and value copy is indeed negligible compared to I/O overhead. However, we cannot say that using getOrPut in this case is wrong, because doing this is intuitive. Its efficiency on the happy path is indeed nothing to criticize.
So I should still consider this a valid use case of getOrPut that defer comptime unreachable cannot cover.
The Fine Article itself offers a useful heuristic here:
Strong Exception Safety
The object state remains as if we didn’t attempt the operation.
Basic Exception Safety
The object is left in a different, but valid state.
No Exception Safety
The object becomes invalid and unsafe to use.
The case being made (and I about three-quarters agree) is that code should prefer strong exception safety. That is, if we agree to not count reservation of memory as mutation (for the sake of discussion), that it is best to reserve all memory and only then mutate.
Second best is to do the minimum amount of useful mutation, while making sure that errdefer unwinds anything so mutated to a correct state, in the event of subsequent errors.
In this case that would be removing the key/value pair where the value is left in an undefined state. .remove(key) is infallible, so this is tractable here.
But unwinding things is harder to get right, so it’s usually better when one doesn’t have to.
I agree with this, and I think that every function in Zig that returns an error union should be assumed to have Strong Exception Safety, unless explicitly documented otherwise.
In other words if you make a function that doesn’t have Strong Exception Safety you should note that in the doc comment. I think we can all agree that should be standard best practice in Zig.
I believe that the combination of two things causes issues with “strong exception safety” that need to be addressed: 1. a state change occurs; 2. an exception occurs.
I think errdefer comptime unreachable ensures strong exception safety by asserting that ‘an exception will not occur’, thereby preventing point 2 from happening.
I have no doubt that in 90% of scenarios, it can achieve this. But my concern is that this approach makes us less sensitive to point 1, ‘a state change has occurred,’ because in scenarios where exceptions are not allowed, strong exception safety is unaffected regardless of whether a state change happens or not.
Whether it is appendAssumeCapacity or getOrPutAssumeCapacity, although they do not report errors themselves, state changes still occur. This usage is merely ‘a measure to prevent exceptions.happen’ I worry that this approach, which frees programmers from having to consider errdefer, will gradually make them ignore state changes.
I think that using errdefer comptime unreachable together with errdefer after a state modification is a good practice. It still reminds programmers that a state change has occurred, ensuring safety for point 1, while errdefer comptime unreachable provides a safety net for point 2.