Add a way to pass additional arguments to an actor constructor

[svetlyak40wt]

I've removed :other-args argument.
Probably it should be returned back for backward compatibility?

[mdbergmann]

Would it be OK for you to not have the queue-size here and instead process it as part of rest? Because I think that to 95% of the use cases the default unbounded queue is being used. It can still be documented as docstring what additional parameters are being accepted.

[svetlyak40wt]

It will work, but I think it is much more convenient to have known arguments listed explicitly, because IDE will suggest their names.

[mdbergmann]

That is true. But I don't want users to be confused what to choose here and whether it is relevant on constructing an actor.

[svetlyak40wt]

Ok, I've removed queue-size from explicit arguments of generic function, but leave this keyarg for a method where it is used.

[mdbergmann]

Is this necessary? IMO it doesn't look great and adds an additional burden to strip keys here (and in the other places.
What happens if the keys are not removed?

[svetlyak40wt]

If these keys are not removed from the REST, they will be passed along to the act:make-actor and will cause error like:

Invalid initialization argument:
  :DISPATCHER"


 0: (SB-PCL::INITARG-ERROR #<STANDARD-CLASS SENTO.DISPATCHER:DISPATCH-WORKER> (:DISPATCHER))
 1: ((:METHOD MAKE-INSTANCE (CLASS)) #<STANDARD-CLASS SENTO.DISPATCHER:DISPATCH-WORKER> :RECEIVE #<FUNCTION SENTO.DISPATCHER::RECEIVE> :INIT N$
 2: ((:METHOD MAKE-ACTOR (T)) #<FUNCTION SENTO.DISPATCHER::RECEIVE> :INIT NIL :DESTROY NIL :STATE NIL :NAME "dispatch(SHARED)-worker-1" :RECEI$
      Locals:
        #:.DEFAULTING-TEMP. = SENTO.DISPATCHER:DISPATCH-WORKER
        SENTO.ACTOR::ALL-ARGS = (:INIT NIL :DESTROY NIL :STATE NIL :NAME "dispatch(SHARED)-worker-1" :RECEIVE #<FUNCTION SENTO.DISPATCHER::REC$
        SENTO.ACTOR::RECEIVE = #<FUNCTION SENTO.DISPATCHER::RECEIVE>
        TYPE = SENTO.DISPATCHER:DISPATCH-WORKER
 3: ((SB-PCL::EMF MAKE-ACTOR) #<unused argument> #<unused argument> #<FUNCTION SENTO.DISPATCHER::RECEIVE> :INIT NIL :DESTROY NIL :STATE NIL :T$
 4: ((LAMBDA NIL :IN SENTO.ACTOR-CONTEXT:ACTOR-OF))
 5: (SENTO.ACTOR-CONTEXT::%CREATE-ACTOR #<SENTO.ACTOR-CONTEXT:ACTOR-CONTEXT {1006010683}> #<FUNCTION (LAMBDA NIL :IN SENTO.ACTOR-CONTEXT:ACTOR$
      Locals:
        CONTEXT = #<SENTO.ACTOR-CONTEXT:ACTOR-CONTEXT {1006010683}>
        CREATE-FUN = #<FUNCTION (LAMBDA () :IN SENTO.ACTOR-CONTEXT:ACTOR-OF) {10061575BB}>
        DISPATCHER-ID = :PINNED
        QUEUE-SIZE = NIL
 6: (SENTO.ACTOR-CONTEXT::%ACTOR-OF #<SENTO.ACTOR-CONTEXT:ACTOR-CONTEXT {1006010683}> #<FUNCTION (LAMBDA NIL :IN SENTO.ACTOR-CONTEXT:ACTOR-OF)$
 7: ((:METHOD SENTO.ACTOR-CONTEXT:ACTOR-OF (SENTO.ACTOR-CONTEXT:ACTOR-CONTEXT)) #<SENTO.ACTOR-CONTEXT:ACTOR-CONTEXT {1006010683}> :RECEIVE #<F$
 8: ((SB-PCL::EMF SENTO.ACTOR-CONTEXT:ACTOR-OF) #<unused argument> #<unused argument> #<SENTO.ACTOR-CONTEXT:ACTOR-CONTEXT {1006010683}> :RECEI$
 9: (SENTO.DISPATCHER:MAKE-DISPATCHER-WORKER 1 #<SENTO.ACTOR-CONTEXT:ACTOR-CONTEXT {1006010683}> :SHARED)
10: ((:METHOD INITIALIZE-INSTANCE :AFTER (SENTO.DISPATCHER:SHARED-DISPATCHER)) #<SENTO.DISPATCHER:SHARED-DISPATCHER ident: SHARED, workers: 0,$
11: ((LAMBDA (SB-PCL::|.P0.| SB-PCL::|.P1.| SB-PCL::|.P2.| SB-PCL::|.P3.|)) #<unavailable argument> #<unavailable argument> #<unavailable argu$
12: (SENTO.DISPATCHER:MAKE-DISPATCHER #<SENTO.ACTOR-CONTEXT:ACTOR-CONTEXT {1006010683}> :SHARED :WORKERS 4 :STRATEGY :RANDOM)
13: (SENTO.ACTOR-SYSTEM::%REGISTER-DISPATCHERS #<SENTO.ACTOR-SYSTEM:ACTOR-SYSTEM config: (SCHEDULER (ENABLED TRUE RESOLUTION 100 MAX-SIZE 500)$
14: (SENTO.ACTOR-SYSTEM:MAKE-ACTOR-SYSTEM (:DISPATCHERS (:SHARED (:WORKERS 4 :STRATEGY :RANDOM))))
15: ((LABELS %TEST-COUNTER-MP-UNBOUNDED :IN "/home/art/.cache/common-lisp/sbcl-2.4.4-linux-x64/home/art/projects/cl-gserver/tests/actor-mp-tes$
16: ((LABELS IT.BESE.FIVEAM::RUN-IT :IN IT.BESE.FIVEAM::RUN-TEST-LAMBDA))

This error is generated because actor class does not have :DISPATCHER initarg.

I've found that in this case we can replace lengthy:

(alexandria:remove-from-plistf rest
                                 :queue-size
                                 :dispatcher
                                 :init
                                 :destroy
                                 :state
                                 :type
                                 :name)

With only:

(alexandria:remove-from-plistf rest
                                 :queue-size
                                 :dispatcher)

or we can remove alexandria:remove-from-plistf completely but pass :allow-other-keys t to the make-instance. But this way we will turn off check of keyword argument and library user might be surprised when hi mistype an argument name and actor's slot will not be filled with the value.

I'd go with a smaller number of excluded argument names but not turn off names checking.

[mdbergmann]

OK

[svetlyak40wt]

Fixed.

[mdbergmann]

Better use await-cond or some such here because we may have the result earlier and can spare some test runtime.
Possibly the 1 sec wait in receive can also be reduced.

[svetlyak40wt]

I've refactored code like this, but it required some changes in complete-p function.

Could you please review if it is ok to change complete-p this way? If so, probably a separate test should added for the cases where future can become completed with error.

What is interesting, I've replaced TELL with ASK and ASK does not raise sento.queue:queue-full-error error as TELL does. Instead it returns a "errored" future. Is this desired behaviour?

[mdbergmann]

ask always returns future. Insofar it consistently also delivers any errors as future result. While it's consistent it has a technical reason: ask spawns an anonymous actor that does the tell (and also waits for response via reply) so that the call to ask can be async.

[mdbergmann]

I've removed :other-args argument. Probably it should be returned back for backward compatibility?

I don't know if this is used by many, if any at all. But we don't know and it may be we break it for some.
I'm using it in other projects and I'll probably have to adapt.
I think we can just make the change and see if someone complains.

[mdbergmann]

That's OK. error result can be considered completed.

[mdbergmann]

Please add a test-case for the changes, and for error-p.

[svetlyak40wt]

Added complete-with-error test.

[mdbergmann]

If you don't have more I'd merge.

[svetlyak40wt]

If you don't have more I'd merge.

Yes, I'm done.