Interpreter change

Please integrate immediately, unless the same change already reached  
the Squeak VMM by way of the Croquet VMM.


'From Croquet1.0beta of 11 April 2006 [latest update: #0] on 17 April  
2006 at 10:29:39 am'!
"Change Set: Socket-latencyFix-ikp
Date: 14 April 2006
Author: Ian Piumarta

Reset nextPollTick to zero in forceInterruptCheck so that  
asynchronous external i/o can obtain an immediate ioProcessEvents."!


!Interpreter methodsFor: 'processes' stamp: 'ikp 4/14/2006 16:30'!
forceInterruptCheck
        "force an interrupt check ASAP"
        interruptCheckCounter := -1000.
        nextPollTick := 0! !

This is a bit uclear, all this is doing is forcing a call to  
ioProcessEvents()

This assumes ioProcessEvents() is doing something interesting that  
you want it to do...
which is?

Hint on the mac all it does is check to see if the screen bits should  
be flush (every 20ms) , and snatch any pending UI events  (every 16ms).
Nothing else. So what else should it be doing?


On 17-Apr-06, at 10:33 AM, Ian Piumarta wrote:

--
========================================================================
===
John M. McIntosh <[hidden email]> 1-800-477-2659
Corporate Smalltalk Consulting Ltd.  http://www.smalltalkconsulting.com
========================================================================
===

Hi John -

The rational of this change is to ensure that when you force an
interrupt check that we query for i/o events as well since the effect of
the interrupt change might be related to i/o. The case in question was
sockets: Without that change, ioProcessEvents would only be called every
200ms if the VM is in a busy loop which limits latency to 200ms in a
case like this:

   socket connectTo: addr port: port.
   [socket isConnected] whileFalse.

(this is a bit unrealistic example but there are many realistic ones)

In order to overcome the latency issue we need to both, signal the VM
that something has happened (we do this via forceInterruptCheck) but we
also need to make sure we look at ioProcessEvents ASAP, which is why the
nextPollTick is being reset. This brings the latency down to
(effectively) nothing as illustrated in the Croquet VM tests.

Note that these changes will likely have *dramatic* positive impacts for
systems running on servers under heavy computational load. Many of the
oddities reported on Unix systems can likely be traced to this very problem.

Cheers,
   - Andreas

John M McIntosh wrote:

On Apr 17, 2006, at 2:04 PM, Andreas Raab wrote:

> Note that these changes will likely have *dramatic* positive  
> impacts for systems running on servers under heavy computational  
> load. Many of the oddities reported on Unix systems can likely be  
> traced to this very problem.

I should clarify that the fix also requires changes in the Unix  
support code for it to be effective.  Recompiling the 3.8 Unix code  
base with the modified forceInterruptCheck() will not make any  
difference to socket performance.  An upcoming 3.9 release will  
include the changes, which are already checked in to SVN.

Cheers,
Ian

On 17-Apr-06, at 2:00 PM, Ian Piumarta wrote:
mmm

ok let me say


ioProcessEvents() is a null call on os-x.

you have some other magic you are expecting it to do?


--
========================================================================
===
John M. McIntosh <[hidden email]> 1-800-477-2659
Corporate Smalltalk Consulting Ltd.  http://www.smalltalkconsulting.com
========================================================================
===

John M McIntosh wrote:
> ioProcessEvents() is a null call on os-x.
>
> you have some other magic you are expecting it to do?

How do you deal with sockets? I thought you were using the Unix socket
code which relies on aio* and which is commonly called in
ioProcessEvents. If not there, where *do* you call it? ;-)

Cheers,
   - Andreas

For some reason lost in time, it's called only in

ioGetNextEvent()


I do think here if you want a specialized call to process socket  
events what about a flag aioPollNeeded and set that to true, then  
check for that in checkForInterrupts()
Otherwise  you end up with ioProcessEvents() being called say oh  
10,000 a second if there are lots of socket events happening? and  
just because ioProcessEvents() handles
sockets doesn't mean it might be doing other things which are  
assuming a low calling rate.

On 17-Apr-06, at 2:19 PM, Andreas Raab wrote:

--
========================================================================
===
John M. McIntosh <[hidden email]> 1-800-477-2659
Corporate Smalltalk Consulting Ltd.  http://www.smalltalkconsulting.com
========================================================================
===

Hi John -

> I do think here if you want a specialized call to process socket events
> what about a flag aioPollNeeded and set that to true, then check for
> that in checkForInterrupts()
> Otherwise  you end up with ioProcessEvents() being called say oh 10,000
> a second if there are lots of socket events happening? and just because
> ioProcessEvents() handles
> sockets doesn't mean it might be doing other things which are assuming a
> low calling rate.

Yes, this could be done but the "minimally invasive" solution of
resetting nextPollTick seems pretty good in my understanding. Mostly
because a) a platform doesn't have to force an interrupt check if it
can't handle 10k queries a second and b) if it's really that slow in a
part of ioProcessEvents it could use a timer or somesuch to not overdo
the polling.

If we would change that, we probably should factor this so that we have
a call to ioProcessInterrupts() being called from checkForInterrupts()
(instead of ioProcessEvents) so that the platform code can decide what
is proper to do in an interrupt check vs. what is proper to do in a
"full" event query. In practice, I'd expect the two to be the same however.

Having said that, let me add that all *I* care about right now, is VMs
that pass the Croquet VM tests ;-) The first solution has already been
proven to do that so I have a clear preference for having that solution
*right now* simply because I've already seen it working.

Cheers,
   - Andreas

On Apr 17, 2006, at 2:30 PM, John M McIntosh wrote:

> For some reason lost in time, it's called only in
>
> ioGetNextEvent()
>
> I do think here if you want a specialized call to process socket  
> events what about a flag aioPollNeeded and set that to true, then  
> check for that in checkForInterrupts()

One obvious condition would be to set the flag when external  
semaphores are registered.  That might be messier than just resetting  
nextPollTick.

> Otherwise  you end up with ioProcessEvents() being called say oh  
> 10,000 a second if there are lots of socket events happening? and  
> just because ioProcessEvents() handles
> sockets doesn't mean it might be doing other things which are  
> assuming a low calling rate.

The question is how to separate things the image/Interpreter know  
most about from things the support knows most about, and keeping the  
assumptions between them as simple as possible.

If the support is doing some expensive operation in ioProcessEvents,  
maybe the support should arrange to limit the frequency of these  
operations rather than trying to communicate something back to the  
Interpreter to affect the frequency indirectly.  OTOH, assuming that  
ioProcessEvents is either (1) expensive or (2) not performing a time-
critical poll of activity (in the absence of any other easy means to  
perform such a poll) is also a bad mistake on the part of the  
Interpreter.

Arranging for forceInterruptCheck to cause an ioProcessEvents at the  
earliest opportunity seems to me to make the fewest assumptions  
across the Interperter/support barrier.  The support code knows what  
it's doing in calling forceIntrCheck, and can take steps to  
compensate for the frequency of ioProcessEvents if necessary.  (The  
nextPollTick logic would, I suspect, be far better in ioProcessEvents  
than in checkForInterrupts.)

If you have a simpler solution I'd be eager to hear it.

Cheers,
Ian

The other thing I'm confused about is that in the past
ioRelinquishProcessorForMicroseconds()

where we would calculate the pending wakeup time based on my thoughts  
of a few years ago then we would
have said aioPoll(realTimeToWait)....

and the aioPoll() logic would do it's thing for the supposed set of  
micro seconds.

However now I see that this has changed to:

        if (!aioPoll(0)) {
                  struct timespec rqtp= { 0, realTimeToWait * 1000*1000 };
                  struct timespec rmtp;
                  while ((nanosleep(&rqtp, &rmtp) < 0) && (errno == EINTR))
                        rqtp= rmtp;
        }

so why the change not to have the select() do the sleep?

what are the conditions that would cause aioPoll() to return 0, thus  
invoking this nanosleep
and if the nanosleep is invoked, would 16 ms pass before servicing a  
socket? if realTimeToWait is 16ms?


On 17-Apr-06, at 2:19 PM, Andreas Raab wrote:

--
========================================================================
===
John M. McIntosh <[hidden email]> 1-800-477-2659
Corporate Smalltalk Consulting Ltd.  http://www.smalltalkconsulting.com
========================================================================
===

On Apr 17, 2006, at 2:56 PM, John M McIntosh wrote:

Because select() cannot guarantee granularity of less than a  
timeslice once it has decided to go to sleep (hence the test just  
above that code for ms < timeslice quantum).  You were sitting in my  
office when we proved this.

> what are the conditions that would cause aioPoll() to return 0

No pending i/o activity on any open descriptors.

> and if the nanosleep is invoked, would 16 ms pass before servicing  
> a socket? if realTimeToWait is 16ms?

Not necessarily.  If your kernel is clever enough to return early  
from nanosleep when a signal is delivered then socket i/o will have  
minimum latency.  (On OSX this appears to be the case: nanosleep can  
return -1 and set errno to EINTR.)

Cheers,
Ian

On 17-Apr-06, at 2:53 PM, Ian Piumarta wrote:

ioProcessEvents had an implied expense which is why it was only  
checked every 500 ms, or what ever.

What I'd suggest here is setting a aioPollNeed flag

have forceaioPoll() that does
        set the aioPollNeed flag
        calls forceInterruptCheck()

in checkForInterrupts()
check aioPollNeed, if true call aioPoll() and reset.


>
> Cheers,
> Ian
>

--
========================================================================
===
John M. McIntosh <[hidden email]> 1-800-477-2659
Corporate Smalltalk Consulting Ltd.  http://www.smalltalkconsulting.com
========================================================================
===

So is aioPoll() ever called with a non-zero value?

On 17-Apr-06, at 3:04 PM, Ian Piumarta wrote:

> Because select() cannot guarantee granularity of less than a  
> timeslice once it has decided to go to sleep (hence the test just  
> above that code for ms < timeslice quantum).  You were sitting in  
> my office when we proved this.

--
========================================================================
===
John M. McIntosh <[hidden email]> 1-800-477-2659
Corporate Smalltalk Consulting Ltd.  http://www.smalltalkconsulting.com
========================================================================
===

On Apr 17, 2006, at 3:08 PM, John M McIntosh wrote:

> So is aioPoll() ever called with a non-zero value?

Yes.  The line after the code you quoted is

   dpy->ioRelinquishProcessorForMicroseconds(ms*1000);

which defers to the active display driver.  When the display is X11  
(for example), the above calls

static sqInt display_ioRelinquishProcessorForMicroseconds(sqInt  
microSeconds)
{
   aioPoll(handleEvents() ? 0 : microSeconds);
   return 0;
}

in sqUnixX11.c.

Cheers,
Ian

Ahh?

so if next wakeup tick  is less than 16ms then call aioPoll(0) then  
nanosleep for upto 16ms ish then call aioPoll(0)  mmm I guess that  
services any interrupt that woke things up.
and if greater than 16ms  you end up sleeping in select() via  
callling aioPoll(waitfor).

mind I didn't look to see what handleEvents() does...  Perhaps you  
never sleep?

Any chance you could fold the nanosleep back into the aioPoll() logic  
instead of making it a special case outside of select logic in aioPoll()



On 17-Apr-06, at 3:21 PM, Ian Piumarta wrote:

--
========================================================================
===
John M. McIntosh <[hidden email]> 1-800-477-2659
Corporate Smalltalk Consulting Ltd.  http://www.smalltalkconsulting.com
========================================================================
===

Hi John -

> What I'd suggest here is setting a aioPollNeed flag
>
> have forceaioPoll() that does
>     set the aioPollNeed flag
>     calls forceInterruptCheck()
>
> in checkForInterrupts()
> check aioPollNeed, if true call aioPoll() and reset.

What for? We can just call ioProcessInterrupts() no matter what. If you
really need to have extra tests for aio you can implement them in the
support code, e.g.,

ioProcessInterrupts() {
   if(aioPollNeeded) aioPoll();
}

and leave the main VM without further modifications (I really dislike
adding such unnecessary extensions to the VM itself - it's up to the
support code to behave as stated, and if the stated behavior is to get
out quickly, well, then the support code should just comply).

Cheers,
   - Andreas

On Apr 17, 2006, at 3:38 PM, John M McIntosh wrote:

> so if next wakeup tick  is less than 16ms then call aioPoll(0) then  
> nanosleep for upto 16ms ish then call aioPoll(0)  mmm I guess that  
> services any interrupt that woke things up.
> and if greater than 16ms  you end up sleeping in select() via  
> callling aioPoll(waitfor).

I couldn't have described it better.

> mind I didn't look to see what handleEvents() does...  Perhaps you  
> never sleep?

handleEvents() returns 1 if any UI events came in (or if the event  
buffer is not empty), otherwise 0 to indicate no UI activity to worry  
about.

In other words, ioRelinquishProc only sleeps if there is no UI  
activity and no descriptor is ready for a pending i/o operation.

> Any chance you could fold the nanosleep back into the aioPoll()  
> logic instead of making it a special case outside of select logic  
> in aioPoll()

Would this be particularly helpful to you?  I never call aioPoll()  
with non-zero from anywhere except relinquishProc, so for me it's  
better being outside aioPoll.  But if you'd find it useful inside  
aioPoll then I don't think moving it there would break anything.

Cheers,
Ian

On 17-Apr-06, at 3:47 PM, Ian Piumarta wrote:

That would be better, then no chance to miss a change later, so if I  
call aioPoll(0) you do the right thing and
if I call aioPoll(timeToWait) then you wait N ms unless I guess an  
event comes in that needs to be processed thus terminating the wait.

--
========================================================================
===
John M. McIntosh <[hidden email]> 1-800-477-2659
Corporate Smalltalk Consulting Ltd.  http://www.smalltalkconsulting.com
========================================================================
===

On Apr 17, 2006, at 3:57 PM, John M McIntosh wrote:

>> Would this be particularly helpful to you?  I never call aioPoll()  
>> with non-zero from anywhere except relinquishProc, so for me it's  
>> better being outside aioPoll.  But if you'd find it useful inside  
>> aioPoll then I don't think moving it there would break anything.
>
> That would be better, then no chance to miss a change later, so if  
> I call aioPoll(0) you do the right thing and
> if I call aioPoll(timeToWait) then you wait N ms unless I guess an  
> event comes in that needs to be processed thus terminating the wait.

Checked-in, but you'll want to call aioSleep() rather than aioPoll().

Cheers,
Ian

k, thank you

On 17-Apr-06, at 5:15 PM, Ian Piumarta wrote:

--
========================================================================
===
John M. McIntosh <[hidden email]> 1-800-477-2659
Corporate Smalltalk Consulting Ltd.  http://www.smalltalkconsulting.com
========================================================================
===