By the way, GCC and Clang have attribute((cleanup)) (which is the same, scope-based clean-up) and have done for over a decade, and this is widely used in open source projects now.
I hate even more that you can call defer in a loop, and it will appear to work, as long as the loop has relatively few iterations, and is just silently massively wasteful.
In a tight loop you'd want your cleanup to happen after the fact. And in, say, an IO loop, you're going to want concurrency anyway, which necessarily introduces new function scope.
Why? Doing 10 000 iterations where each iteration allocates and operates a resource, then later going through and freeing those 10 000 resources, is not better than doing 10 000 iterations where each iteration allocates a resource, operates on it, and frees it. You just waste more resources.
> And in, say, an IO loop, you're going to want concurrency anyway
This is not necessarily true; not everything is so performance sensitive that you want to add the significant complexity of doing it async. Often, a simple loop where each iteration opens a file, reads stuff from it, and closes it, is more than good enough.
Say you have a folder with a bunch of data files you need to work on. Maybe the work you do per file is significant and easily parallelizable; you would probably want to iterate through the files one by one and process each file with all your cores. There are even situations where the output of working on one file becomes part of the input for work on the next file.
Anyway, I will concede that all of this is sort of an edge case which doesn't come up that often. But why should the obvious way be the wrong way? Block-scoped defer is the most obvious solution since variable lifetimes are naturally block-scoped; what's the argument for why it ought to be different?
And to draw a surface to the screen, you need to create an SDL texture from it. If that's all you want to do, you can then destroy the SDL surface.
So you could imagine code like this:
strings := []string{"Lorem", "ipsum", "dolor", "sit", "amet"}
stringTextures := []SDLTexture{}
for _, s := range strings {
surface := RenderTextToSurface(s)
defer surface.Destroy()
stringTextures = append(stringTextures, surface.CreateTexture())
}
Oops, you're now using way more memory than you need!And what would be the benefit? You save up to one malloc and free per string you want to render, but text rendering is so demanding it completely drowns out the cost of one allocation.
If you were dynamically changing the malloc allocation size on each iteration then you have a case for a growable buffer to do the same, but in that case you would already have all the complexity of which you speak as required to support a dynamically-sized malloc.
Granted, you could do a pre-pass to find the largest string and allocate enough memory for that once, then use that buffer throughout the loop.
But again, what do you gain from that complexity?
Impossible without knowing how much to allocate, which you indicate would require adding a bunch of complexity. However, I am willing to chalk that up to being a typo. Given that we are now calculating how much to allocate on each iteration, where is the meaningful complexity? I see almost no difference between:
while (next()) {
size_t size = measure_text(t);
void *p = malloc(size);
draw_text(p, t);
free(p);
}
and void *p = NULL;
while (next()) {
size_t size = measure_text(t);
void *p = galloc(p, size);
draw_text(p, t);
}
free(p);> Impossible without knowing how much to allocate
But we do know how much to allocate? The implementation of this example's RenderTextToSurface function would use SDL functions to measure the text, then allocate an SDL_Surface large enough, then draw to that surface.
> I see almost no difference between: (code example) and (code example)
What? Those two code examples aren't even in the same language as the code I showed.
The difference would be between the example I gave earlier:
stringTextures := []SDLTexture{}
for _, str := range strings {
surface := RenderTextToSurface(str)
defer surface.Destroy()
stringTextures = append(stringTextures, surface.CreateTexture())
}
and: surface := NewSDLSurface(0, 0)
defer surface.Destroy()
stringTextures := []SDLTexture{}
for _, str := range strings {
size := MeasureText(s)
if size.X > surface.X || size.Y > surface.Y {
surface.Destroy()
surface = NewSDLSurface(size.X, size.Y)
}
surface.Clear()
RenderTextToSurface(surface, str)
stringTextures = append(stringTextures, surface.CreateTextureFromRegion(0, 0, size.X, size.Y))
}
Remember, I'm talking about the API to a Go wrapper around SDL. How the C code would've looked if you wrote it in C is pretty much irrelevant.I have to ask again though, since you ignored me the first time: what do you gain? Text rendering is really really slow compared to memory allocation.
We were talking about using malloc/free vs. a resizable buffer. Happy to progress the discussion towards a Go API, however. That, obviously, is going to look something more like this:
renderer := SDLRenderer()
defer renderer.Destroy()
for _, str := range strings {
surface := renderer.RenderTextToSurface(str)
textures = append(textures, renderer.CreateTextureFromSurface(surface))
}
I have no idea why you think it would look like that monstrosity you came up with.No. This is a conversation about Go. My example[1], that you responded to, was an example taken from a real-world project I've worked on which uses Go wrappers around SDL functions to render text. Nowhere did I mention malloc or free, you brought those up.
The code you gave this time is literally my first example (again, [1]), which allocates a new surface every time, except that you forgot to destroy the surface. Good job.
Can this conversation be over now?
To anyone used to SDL, your proposed API is extremely surprising.
It would've made your point clearer if you'd explained this coupling between SDL_Renderer and text rendering in your original post.
But yes, I concede that if there was any reason to do so, putting a scratch surface into your SDL_Renderer that you can auto-resize and render text to would be a solution that makes for slightly nicer API design. Your SDL_Renderer now needs to be passed around as a parameter to stuff which only ought to need to concern itself with CPU rendering, and you now need to deal with mutexes if you have multiple goroutines rendering text, but those would've been alright trade-offs -- again, if there was a reason to do so. But there's not; the allocation is fast and the text rendering is slow.
Aside from my failure in dealing with the hardest problem in computer science, how would you improve the intent of the API? It is clearly improved over the original version, but we would do well to iterate towards something even better.
But that 0.1 millisecond to render a string is an eternity compared to the time it takes to allocate some memory, which might be on the order of single digit microseconds. Saving a microsecond from a process which takes 0.1 milliseconds isn't noticeable.
It might be preferable to create a font atlas and just allocate printable ASCII characters as a spritesheet (a single SDL_Texture* reference and an array of rects.) Rather than allocating a texture for each string, you just iterate the string and blit the characters, no new allocations necessary.
If you need something more complex, with kerning and the like, the current version of SDL_TTF can create font atlases for various backends.
P.S. If you have enough files you don't want to try to open them all at once — Go will start creating more and more threads to handle the "blocked" syscalls (open(2) in this case), and you can run out of 10,000 threads too
If you have a legitimate reason for doing something unusual, it is fine to have to use the tools unusually. It serves as a useful reminder that you are purposefully doing something unusual rather than simply making a bad design choice. A good language makes bad design decisions painful.
What you propose is not a bad solution, but don't come here and pretend it's the only reasonable solution for almost all situations. It's not. Sometimes, you want each work item to be a list of files, if processing one file is fast enough for synchronisation overhead to be significant. Often, you don't have to care so much about the wall clock time your loop takes and it's fast enough to just do sequentially. Sometimes, you're implementing a non-important background task where you intentionally want to only bother one core. None of these are super unusual situations.
It is telling that you keep insisting that any solution that's not a one-file-per-work-item work queue is super strange and should be punished by the language's design, when you haven't even responded to my core argument that: sometimes sequential is fast enough.
Keep insisting? What do you mean by that?
> when you haven't even responded to my core argument that: sometimes sequential is fast enough.
That stands to reason. I wasn't responding to you. The above comment was in reply to nasretdinov.
for filename := range workQueue {
fp, err := os.Open(filename)
if err != nil { ... }
defer fp.Close()
// do work
}
Which would have the same exact problem :) for _, filename := range files {
queue <- func() {
f, _ := os.Open(filename)
defer f.Close()
}
}
or more realistically, var group errgroup.Group
group.SetLimit(10)
for _, filename := range files {
group.Go(func() error {
f, err := os.Open(filename)
if err != nil {
return fmt.Errorf("failed to open file %s: %w", filename, err)
}
defer f.Close()
// ...
return nil
})
}
if err := group.Wait(); err != nil {
return fmt.Errorf("failed to process files: %w", err)
}
Perhaps you can elaborate?I did read your code, but it is not clear where the worker queue is. It looks like it ranges over (presumably) a channel of filenames, which is not meaningfully different than ranging over a slice of filenames. That is the original, non-concurrent solution, more or less.
// Spawn workers
for _ := range 10 {
go func() {
for path := range workQueue {
fp, err := os.Open(path)
if err != nil { ... }
defer fp.Close()
// do work
}
}()
}
// Iterate files and give work to workers
for _, path := range paths {
workQueue <- path
}Now we know why it was painful. What is interesting here is that the pain wasn't noticed as a signal that the design was off. I wonder why?
We should dive into that topic. I suspect at the heart of it lies why there is so much general dislike for Go as a language, with it being far less forgiving to poor choices than a lot of other popular languages.
But one does still have to be mindful if they want to write software productively. Using a "super duper generic and extensible" solution means that things like error propagation is already solved for you. Your code, on the other hand, is going to quickly become a mess once you start adding all that extra machinery. It didn't go unnoticed that you conveniently left that out.
Maybe that no longer matters with LLMs, when you don't even have to look the code and producing it is effectively free, but LLMs these days also understand how defer works so then this whole thing becomes moot.
Sometimes we need block scoped cleanup, other times we need the function one.
You can turn the function scoped defer into a block scoped defer in a function literal.
AFAICT, you cannot turn a block scoped defer into the function one.
So I think the choice was obvious - go with the more general(izable) variant. Picking the alternative, which can do only half of the job, would be IMO a mistake.
You kinda-sorta can by creating an array/vector/slice/etc. of thunks (?) in the outer scope and then `defer`ing iterating through/invoking those.
In Golang if you iterate over a thousand files and
defer File.close()
your OS will run out of file descriptorsSeriously, why is default ulimit on file descriptors on Linux measly 1024?