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

Global process counter and table #100

Merged
merged 5 commits into from May 21, 2019
Merged

Conversation

KronicDeth
Copy link
Collaborator

Changelog

Enhancements

  • Global local process counter, so getting pids for processes no long needs to go through the Process's Environment.
  • Process by pid map is moved to process::local. Processes are always passed as immutable references and interior mutability is used to support global process map trait requirements and to support smaller locks for future work.
  • Make all process arenas private to eliminate the needs for Arcs around arenas when it is know the callers would always be in the same process.
    • Don't pass the arenas outside of impl Process to eliminate need for RefCell.
  • Don't fake Send and Syn on Process.
    • Switch from im_rc to im so that the map arena is Send.
    • Make heap::Binary Send to override *const u8 in bytes not being Send. heap::Binary are immutable after create, so they can be safely Send even with the raw pointer.
    • Wrap all arenas in Process in Mutex. In most cases there will only be one be accessed by the one scheduler thread. The Mutex is only needed to make Sync derivation happy and potentially during scheduler hand-off.

Incompatible Changes

  • Eliminate Environment as anything that would have been in the Environment will not be global to eliminate unnecessary pointer hops.

Getting pids for processes no longer needs to go through the Process's
Environment.
Process by pid map is moved to process::local.  Processes are always
passed as immutable pointers and interior mutability is used to support
global process map trait requirements and to support smaller locks for
future work.
@KronicDeth KronicDeth added the enhancement An enhancement to existing functionality or new functionality label Apr 23, 2019
@KronicDeth KronicDeth self-assigned this Apr 23, 2019
@bitwalker
Copy link
Collaborator

One thing I think we'll want to steal from BEAM is the way process locks are designed (i.e. the hierarchical locking system). If you need to access more than one locked field of the process, it is trivially easy to deadlock without that design. Thoughts?

@KronicDeth
Copy link
Collaborator Author

I thought you were supplying the hierarchical lock system as part of your GC work because It wouldn’t have the arenas anymore. If you’re not doing that part, I can switch the locks to be nested on top of this. This was doing just enough to make register/2 work on top of it.

@bitwalker
Copy link
Collaborator

@KronicDeth I am supplying the actual locking primitives with the allocator framework, but the hierarchical locking system for processes is more general than allocation/GC, and I've been trying to keep my hands off the process stuff until I get allocation finished. You are right that the arenas are going away, but the locks are still needed for things that are otherwise part of the PCB (things like sending messages, scheduling, etc.). Probably not a change that is needed for this PR per se, but figured I would bring it up, since it seems to be related.

@KronicDeth
Copy link
Collaborator Author

Looking at erl_process_locl.h, I think all the mutexes would fall under the main lock, so I probably should move them into a substruct with that one mutex and only have the pid be outside of it as the pid is read-only.

@bitwalker
Copy link
Collaborator

Yeah that sounds like the way to go to me for now, once we add in some of the other capabilities, then the more complex locking setup can be implemented. That specific lock design is not something I have implemented as part of the allocator framework, since it is pretty unique to this specific area of the system, and I wasn't sure how much we would have in common with BEAM as things evolved, but I suspect we will want to copy it - it is fairly complex, but is one area that the OTP team put a ton of engineering effort in order to make things performant (on multithreaded systems anyway)

@bitwalker bitwalker merged commit c701d5d into max_2 May 21, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement An enhancement to existing functionality or new functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants