-
Notifications
You must be signed in to change notification settings - Fork 1.5k
Investigate greedy GC breaking some test #2185
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maint 8000 ainers 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
Comments
The current investigation is around coincident module to provide it a way to either reference count per thread, an @ntoll suggestion, or simply never collect globally reachable namespaces or classes. |
This works already as solution to our tests issue but I am not 100% sure it's the best we can do ... it's surely the easiest thing to do and, as Proxies are fairly light anyway, maybe worth merging it regardless the possible better approach: WebReflection/coincident#49 |
BTW, the issue is basically mostly only cause by ourselves in our tests because having both MicroPython and Pyodide in a worker is not really what we should recommend or suggest. It is, however, true that the terminal case or the PyEditor makes this use case trivial to create but it's also true that these would rarely execute simultaneously unless forced to do so ... I am pretty confident that change in coincident would already mitigate the concurrent GC hazard and make things better for the time being but I am also convinced I need to fundamentally change some logic in there to allow some reference counting based architecture for multiple workers randomly doing similar things that results into similar references on the main ... this might take some time though, and unfortunately is not even that simple to test (but I'll find a way 'cause I want to make coincident rock solid for all scenarios). |
What happened?
After latest tests update and latest playwright module landed, we've seen sketchy results and it looks like coincident receives GC calls to free local references even if these are used later on in the program.
A polyscript change was required to fix one test but now there's another test failing mostly only with playwright, although I can reproduce the issue via rage-clicking GC button in the performance devtools' panel to see the error myself, which is half good-news (I have a way to investigate that).
Although workers / main dance is fully orchestrated via numbers and it's not really easy to debug/understand what's going on, meaning:
This is probably just a placeholder to allow MR to update various libraries but I think it has top priority, although it is not nearly as easy as it sounds to be fixed ... let's hope I manage in a way that is robust and surprises-less.
What browsers are you seeing the problem on? (if applicable)
No response
Console info
No response
Additional Context
No response
The text was updated successfully, but these errors were encountered: