Erlang-style process primitives

ASSIGNED
Assigned to

Status

--
enhancement
ASSIGNED
9 years ago
7 years ago

People

(Reporter: lhansen, Assigned: kpalacz)

Tracking

unspecified
Future
Bug Flags:
flashplayer-injection -
flashplayer-qrb +
flashplayer-bug -

Details

(Whiteboard: has-patch)

Attachments

(3 attachments, 17 obsolete attachments)

52.07 KB, patch
edwsmith
: review+
Details | Diff | Splinter Review
21.27 KB, patch
edwsmith
: review+
Details | Diff | Splinter Review
867.19 KB, patch
Details | Diff | Splinter Review
(Reporter)

Description

9 years ago
Created attachment 374933 [details] [diff] [review]
v1, pthreads only for now.  Lightly tested, appears to work.

Code to implement Erlang-style shared-nothing message passing on Tamarin, using one real thread per process in a shared memory where the block manager (GCHeap) is shared but each thread is running a separate VM instance.

More elaborate models, eg Google Gears style worker pools, can be built on top of this.

Documentation in shell/Process.as, code in shell/ProcessClass.{h,cpp}.

Note, you must manually rebuild the shell toplevel ABC code if you want to play with this patch.
(Reporter)

Comment 1

9 years ago
Created attachment 374934 [details]
crude example, part 1
(Reporter)

Comment 2

9 years ago
Created attachment 374935 [details]
crude example, part 2
(Reporter)

Comment 3

9 years ago
Created attachment 374936 [details]
crude example, part 3

part 1 is mainloop.as, which is included into part 2 (master.as) and part 3 (slave.as).  compile the master and the slave, then start the shell with master.abc.  ^C to exit.
(Reporter)

Updated

9 years ago
Attachment #374936 - Attachment mime type: application/octet-stream → text/plain
(Reporter)

Comment 4

9 years ago
Oops.  The patch appears to be non-functional following a merge and the test cases are actually slightly off (they use System. rather than Process. for the methods); I tested the wrong executable /and/ the wrong ABCs (sorry)...  So consider the code to be illustrative rather than running at this point.
(Reporter)

Comment 5

9 years ago
Created attachment 375002 [details] [diff] [review]
v2, pthreads only for now.  Better tested :-)
Attachment #374933 - Attachment is obsolete: true
(Reporter)

Comment 6

9 years ago
Created attachment 375003 [details]
Crude example, reworked
Attachment #374934 - Attachment is obsolete: true
Attachment #374935 - Attachment is obsolete: true
Attachment #374936 - Attachment is obsolete: true
(Reporter)

Updated

9 years ago
Target Milestone: --- → Future
(Reporter)

Updated

9 years ago
Assignee: nobody → lhansen
Status: NEW → ASSIGNED
(Reporter)

Comment 7

9 years ago
Created attachment 422585 [details] [diff] [review]
v3 == v2 ported to redux 3583.  pthreads only.
Attachment #375002 - Attachment is obsolete: true
(Reporter)

Comment 8

9 years ago
To do before any kind of review:

- move processClass from avmshell::ShellCore to avmplus::Toplevel
- move Process.as, ProcessClass.h, ProcessClass.cpp from shell/ to core/
- disentangle the avmshell dependencies is the process code, move the support
  into ShellCore and/or AvmCore
- fix the FIXMEs
(Reporter)

Comment 9

9 years ago
User branch on asteam: /users/lhansen/redux-multicore
(Reporter)

Comment 10

9 years ago
Obvious missing things now:

- a proper feature in avmfeatures.{as,h} instead of the hackery in avmbuild.h
  to control whether it's turned on
- probably #ifdefs around some of the changes in core/
- at least test compile on linux to ensure that it's not hopelessly
  mac-centric
- at least reviews sufficient to convince ourselves we're not breaking any
  current functionality
(Reporter)

Comment 11

9 years ago
Linux compile fixes are in: redux-multicore changeset:   4136:ef0fe571f7c1
(Reporter)

Comment 12

9 years ago
Feature fixes are in: redux-multicore changeset:   4137:d71e8f35d875
(Reporter)

Updated

9 years ago
Attachment #375003 - Attachment is obsolete: true
(Reporter)

Updated

9 years ago
Attachment #422585 - Attachment is obsolete: true
(Reporter)

Comment 13

9 years ago
Created attachment 434690 [details] [diff] [review]
/test/multicore

More FYI than really a review request, but I figure I should have formal approval before landing.
Attachment #434690 - Flags: review?(kpalacz)
Attachment #434690 - Flags: review?(edwsmith)
(Reporter)

Comment 14

9 years ago
Created attachment 434693 [details] [diff] [review]
/doc/multicore

Again, FYI but formal approval requested.
Attachment #434693 - Flags: review?(kpalacz)
Attachment #434693 - Flags: review?(edwsmith)
(Reporter)

Comment 15

9 years ago
Created attachment 434696 [details] [diff] [review]
Process and Shm objects

There's a peculiarity with these in that they are present even when we are not building with process support.  The reason is that that's how our builtin.py step works: there's no conditional generation of the builtins depending on what preprocessor macros are in effect, and since there are native methods the generated .cpp files will reference the Process and Shm types.  It's not a big deal but we might want to address this by and by.
Attachment #434696 - Flags: review?(kpalacz)
Attachment #434696 - Flags: review?(edwsmith)
(Reporter)

Comment 16

9 years ago
Created attachment 434697 [details] [diff] [review]
Changes to /core and /shell
Attachment #434697 - Flags: review?(kpalacz)
Attachment #434697 - Flags: review?(edwsmith)
(Reporter)

Comment 17

9 years ago
In addition to these patches, new files must be added to Xcode and Visual Studio project and core/builtin.py and core/avmfeatures.as must be run to generate sundry updated files.
(Reporter)

Updated

9 years ago
Whiteboard: Has patch
(Reporter)

Updated

9 years ago
Assignee: lhansen → kpalacz

Comment 18

9 years ago
[doc,test]/multicore should probably be renamed "processes"

We got bitten by adding Vector to the empty package, and Process/Shm are possibly likely to collide too.   The workaround for Vector was to put them in the __AS3__.vec package, which ASC opens implicitly based on project/commandline settings.

(more soon)

Comment 19

9 years ago
BuiltinTraits.cpp/h, Toplevel.cpp/h, avmplus.h - need VMCFG guards around new classes?

If its infeasible to exclude new Process/Shm classes from builtin.abc when the process feature is disabled, then i suggest moving them to shell_toplevel.abc (without any namespace changes).  just dont want them to leak into shipping code accidentally.  maybe: add a commandline switch to core/builtin.py?

As a first pass, that's all I saw that would need tweaking before landing in tamarin-redux.  WIll review again in more detail soon.  (R+, adding F?edwsmith for the second review)

Updated

9 years ago
Attachment #434690 - Flags: review?(edwsmith) → review+

Updated

9 years ago
Attachment #434693 - Flags: review?(edwsmith) → review+

Updated

9 years ago
Attachment #434696 - Flags: review?(edwsmith) → review+

Updated

9 years ago
Attachment #434697 - Flags: review?(edwsmith) → review+

Updated

9 years ago
Attachment #434697 - Flags: feedback?(edwsmith)
(Reporter)

Comment 20

9 years ago
Regarding isolating Process and Shm, I agree this is a concern but it is probably going to be a pain in practice because they're somewhat tightly intertwined with the VM core.  (It may also be that they need to go into a new package, flash.concurrency or somesuch.)  Not saying it can't happen, but that they were originally in Shell and were moved into Core to clear up coupling issues.

Comment 21

9 years ago
I buy that.  Do we agree we at least need some way to control them based on the process feature being enabled?  In BuiltinTraits/Toplevel/avmplus.h, ifdef is good enough.  in builtins.cpp/h/abc, we sort of run out of steam.

just brainstorming:

* as3 configuration variables could enable/disable them, but based on what?  avmfeatures.as would need to generate something that builtin.py reads and passes to asc

* builtin.py could compile the builtins both ways, and include both blobs, e.g. generate this:
builtin.cpp:
 #ifdef VMCFG_PROCESSES
    /* everything from builtin.cpp with them enabled */
 #else
    /* everything from builtin.cpp with them disabled */
 #endif

it may seem gross, but its only a couple lines of python, probably.
and it  doesn't scale to all features, but thats okay for the medium term.
nativegen.py does something like this to build all the thunks two ways, controlled by an #ifdef, without having to re-run nativegen twice.
(Reporter)

Comment 22

9 years ago
Well, doing nothing is OK for the medium term.  After all it's redux, it won't ship in anything until next year :-)
(Reporter)

Updated

8 years ago
Attachment #434690 - Flags: review?(kpalacz)
(Reporter)

Updated

8 years ago
Attachment #434693 - Flags: review?(kpalacz)
(Reporter)

Updated

8 years ago
Attachment #434696 - Flags: review?(kpalacz)
(Reporter)

Updated

8 years ago
Attachment #434697 - Flags: review?(kpalacz)
Attachment #434697 - Flags: feedback?(edwsmith)
(Assignee)

Comment 23

8 years ago
Created attachment 447871 [details] [diff] [review]
Significant reorganization of the API and implementation - work in progress.

This is a snapshot of  work in progress, early feedback from anyone interested welcome. An the API front, Process objects encapsulate processes, and MessageChannels represent bidirectional communication between two processes. Communication is synchronous (no queuing) and passing of non-primitive data types are not supported yet.
Attachment #434696 - Attachment is obsolete: true
Attachment #434697 - Attachment is obsolete: true
(Reporter)

Comment 24

8 years ago
(In reply to comment #23)
> Created an attachment (id=447871) [details]
> Significant reorganization of the API and implementation - work in progress.
> 
> This is a snapshot of  work in progress, early feedback from anyone interested
> welcome. An the API front, Process objects encapsulate processes, and
> MessageChannels represent bidirectional communication between two processes.
> Communication is synchronous (no queuing) and passing of non-primitive data
> types are not supported yet.

On a style note, do not use plain 'new' and 'delete' - they're not allowed in the player.  Use the mmfx_new etc macros.
(Assignee)

Comment 25

8 years ago
Created attachment 453289 [details] [diff] [review]
Updated work in progress.

Moved API classes to shell, harmonized with new code in the patch for bug 555765, various small fixes.
Attachment #447871 - Attachment is obsolete: true
(Assignee)

Updated

8 years ago
Blocks: 555765
(Assignee)

Updated

8 years ago
No longer blocks: 555765
Depends on: 555765
(Assignee)

Comment 26

8 years ago
Created attachment 455772 [details] [diff] [review]
More work in progress
Attachment #453289 - Attachment is obsolete: true
(Assignee)

Comment 27

8 years ago
Created attachment 481092 [details] [diff] [review]
work in progress, rebased
Attachment #455772 - Attachment is obsolete: true
(Assignee)

Comment 28

8 years ago
Created attachment 485068 [details] [diff] [review]
reworked and rebased
Attachment #481092 - Attachment is obsolete: true
(Assignee)

Comment 29

8 years ago
Created attachment 486711 [details] [diff] [review]
Ported to the cross-platform c++ concurrency abstractions from bug 555765
Attachment #485068 - Attachment is obsolete: true
(Assignee)

Comment 30

8 years ago
Created attachment 487588 [details] [diff] [review]
update

Rebased, cleaned up, isolate termination fixes, MessageChannel improvements.
Attachment #486711 - Attachment is obsolete: true
(Assignee)

Comment 31

8 years ago
Created attachment 492850 [details] [diff] [review]
another update
Attachment #487588 - Attachment is obsolete: true
(Assignee)

Comment 32

8 years ago
Created attachment 510689 [details] [diff] [review]
outsanding changes

diff wrt 5560:fd3ee180e90e
Attachment #492850 - Attachment is obsolete: true

Comment 33

7 years ago
This would help me so much, I have a six-core machine, and sometimes when running intensive canvas websites the browser maxes out a core, but only one core.

Comment 34

7 years ago
Lars, Krzysztof, is this still relevant or obsolete given isolates work.
Flags: flashplayer-qrb+
Flags: flashplayer-injection-
Flags: flashplayer-bug-
Whiteboard: Has patch → has-patch
You need to log in before you can comment on or make changes to this bug.