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

[osx] Avoid the use of AutoreleasePool #1941

Closed
kvark opened this issue Apr 16, 2018 · 7 comments
Closed

[osx] Avoid the use of AutoreleasePool #1941

kvark opened this issue Apr 16, 2018 · 7 comments

Comments

@kvark
Copy link
Member

kvark commented Apr 16, 2018

Related to #1619 and #1779

It's unfortunate to have the autorelease pool managed by our examples. Need to investigate if we can completely automate it.
cc @grovesNL @scoopr @JohnColanduoni

@scoopr
Copy link
Contributor

scoopr commented Apr 16, 2018

If you really want to handle it within gfx, then I suppose the choices are

  1. the first opportune moment for creating a autorelease pool is the backend Instance creation. But I don't think there are other good natural "blocks" of code to surround autoreleasepool, and you would end up with a heuristic like "drain on present", which might get ugly and interact badly with other code that handles autorelease pools. Oh and it wouldn't help with multiple threads. At all.
  2. Wrap all functions that do objective c calls within a autorelease pool object. Incidentally that seems to be what, eg. libSDL is doing, and I guess it shouldn't be all that bad performance wise, as gfx apis shouldn't need to be called in loops of high iteration counts. It should need only to be added user facing entrypoints, not every function, but will that be easy enough to track?
  3. Not really handle it. I'm kind of the mind that it's the job of whoever handles the runloop, but as wrapping libs usually present you with the minimal tools to build your own instead of a managed runloop, then it gets inconvenient in all kinds of example code etc.

When I started writing this, I was leaning on 1. with some hesitance, but going through the downsides while writing, 2. seems to be the safe bet, it's always correct, and the downsides shouldn't be that bad.

I guess it would be nice to have a helper, like fn with_autorelease_pool<F:FnOnce>(func:F), or perhaps a struct with impl Drop. I suppose it could even check a thread boolean if we had already created pool and not do additional ones, but not sure if it is worth the cost. It could also be feature-flagged, so if I'd rather want to handle it myself, the gfx side autorelease pools would just vanish...

It's a bit late, I'm rambling :P

@grovesNL
Copy link
Contributor

grovesNL commented Apr 16, 2018

Thanks for the great summary @scoopr!

As mentioned on Gitter the performance of choice 2 seems like it could be fine, based on some performance tests (like this one). How did you know that libSDL does this, just by glancing through the source code?

@kvark
Copy link
Member Author

kvark commented Apr 17, 2018

I've got some rich insights from our graphics team (@jrmuizel and @mstange):

  • cocoa does have an autorelease pool by default
  • it resets it in the main event loop
  • our examples don't take the benefit of it because we use winit, and it polls events from Cocoa instead of registering a hander in the event loop

Thus, we might be seeing a lower level solution here, potentially. Some preliminary ideas:

  • A) go with run_forever, assuming it registers in Cocoa main loop. Make a timer to run 60fps and trigger an event for us to render.
  • B) post a message every time we are rendering, making this basically "on_idle" function
  • C) dive into winit to see if there is anything else we can do

Also cc @tomaka @francesca64

@francesca64
Copy link
Contributor

On macOS, the implementation of run_forever is almost completely identical to poll_events, so option A wouldn't help. We basically try to force Cocoa to conform to the expectations of winit's design (calling it from our event loop rather than it calling us from its event loop), which has unsurprisingly led to difficult problems.

@willglynn knows a lot more about Cocoa than I do, so can hopefully offer more concrete advice on how to proceed.

@willglynn
Copy link

I don't think winit can help with its current design. Right now, neither winit nor MacOS are underneath gfx in the call stack. If you want an autorelease pool, you're on your own.

My proposed solution to event loop problems in rust-windowing/winit#237 involves coroutines: user code runs on the usual stack, MacOS event loop runs on a different stack, and they cooperatively yield into each other. This addresses the mismatch where both APIs expect to call into the other, but I had not considered how this would interact with autorelease pools. (The only Objective-C objects winit needs to juggle are incoming events, and manual retain+release is sufficient there.) Autorelease pools are implemented using a thread-local stack of pointers which normally follows the call stack, but… winit would be switching the call stack. I think this solution means that autorelease pools would be unavailable for user code on the UI thread, since each poll_events() or run_forever() call is actually a return into MacOS on a separate stack.

I've come to believe that winit needs a more drastic shift, where the user must surrender their thread and operate solely in callbacks. I don't know if I've convinced anyone else, and I'm not currently pursuing that design, but such a change would put MacOS at the bottom of the stack, then winit, then you. This arrangement would eliminate the need for coroutines on MacOS, solve similar problems on basically every other winit platform, and have the side effect of making autorelease pools work automatically.

@scoopr
Copy link
Contributor

scoopr commented Apr 18, 2018

Then you are only left with the problem when you are not using winit at all because you didn't need a window for your bitcoin mining compute jobs ;)

I believe it has been perfectly okay to run the cocoa runloop on your own terms, there is just more stuff to take care of (like autorelease pools), and I guess thats how many of the wrapper libs like sdl/glfw etc. work. But that is actually not really possible on iOS (and android as well), and only way to run code is through platform provided hooks, there might be workarounds with separate threads.

I suppose winit could provide an optional run_forever(event_callback) and have it send Window::Refresh events with DisplayLink. Examples etc. could be changed to that model, but the "traditional" desktop model would still be available.

@willglynn
Copy link

@scoopr As you note, mobile platforms don't fit the "you call the runloop" model. Web doesn't fit either – and in fact web requires both winit and the user's code to return entirely to the browser before receiving more events. Resizing windows on both MacOS and Windows causes the platform to enter an internal runloop from which winit cannot return, but from which winit could still deliver events.

"The runloop calls you" works everywhere.

bors bot added a commit that referenced this issue Apr 26, 2018
1971: Use autorelease pools internally in Metal backend r=grovesNL a=kvark

Fixes #1941
PR checklist:
- [x] `make` succeeds (on *nix)
- [x] `make reftests` succeeds
- [x] tested examples with the following backends: metal


Co-authored-by: Dzmitry Malyshau <kvarkus@gmail.com>
@bors bors bot closed this as completed in #1971 Apr 26, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants