62125 - Web2 framework: triggerServerEvent has built in 50msec delay

In 2020R1.1, the JavaSrcript code to send an event from client back to server is:

        function triggerServerEvent(e, t, o=null) {
            if ("" == e)
                return;
            let n = new XojoAjaxMessage;
            n.controlID = e, n.eventName = t, n.data = o, queuedCommands.push(n), 
            msgQueueTimer = setTimeout(sendServerEvents, 50)
        }

The line "setTimeout(sendServerEvents, 50) " calls the function sendServerEvents() after a 50 msec delay.

50msec is about the ping time between Los Angeles and Atlanta, and is a noticeable amount - for example, fast twitch gamers pay good money to avoid delays of 5 to 10 milliseconds.

Adding this delay to every server event just slows things down. Web2 is so much faster than web 1 in most other areas, this is a shame.

Suggest it be changed to 0 or 1 msec.

Feature Request: <https://xojo.com/issue/62125>

4 Likes

I’ve added code to the case that implements an override, swapping out the triggerServerEvent function with an identical one with a 1 millisecond delay.

In brief testing so far, it works, and “feels snappier”.

1 Like

Why was there a delay in there in the first place?

That’s a very good question. One hopes that it’s not a workaround for some other bug :slight_smile: I’m putting this into production with a 60 user system, and i’ll report back in a few days after they hammer on it.

4 Likes

@Mike_D interesting discovery, thank you! Let’s hope it is just a forgotten residual from early development ;-). I implemented the change into a non critical side with approx. 50 users. No load on Sundays, I will see tomorrow and the following days and will report.

So far, everything works fine. If it a delay is really needed, it most likely won’t be needed in all circumstances, and then I would argue for a feature request, to being able to adapt this value in the session context.

I’m sure it’s there to collect messages and send in bulk rather than sending tons of small packets to the server in a constant stream which could overwork the server app in a large multi-user environment. I would be very careful about overriding anything in the framework, especially something like this that contributes to stability.

1 Like

Fully agree, I would never recommend (nor do it) for a productive system which needs to be rock solid, as long as Xojo has not commented on it. Though 50 ms seems to be quite high as a default value. I like Mike’s console output to warn everyone that the hack is active :wink: .

On a side note: I’m really surprised that it doesn’t break anything. As you I understood that one of the concept (and performance gains) of Web2 is to collect requests and reduce the client and server hops (of Web1). But similar to @Mike I so far only notice a “snappier” feeling, no other bad consequences yet.

But I’m all prepared for a crying fit tomorrow morning and I have my tissues at hand. Well, most likely Greg will give us a hiding very soon :slight_smile: .

I am pretty sure that every event is sent using a separate XMLHTTPrequest, and therefore there is really no benefit to sending more of them after a delay. They aren’t combined in any meaningful way.

Also, web2 doesn’t send very many events in the first place, so it’s not like there is a lot of chatter like there was in web1.

1 Like

The messages aren’t combined into a single packet, but the intention appears to be to stall sending until a 50ms pause in queuing. Although, it doesn’t look like the framework is ever clearing the timeout set in triggerServerEvent, only overwriting the variable holding it’s ID (msgQueueTimer), so it would fire again anyway.

1 Like

To update: I’ve had this fix in production for over 9 months, and it’s stable and helps performance. I’m not sure what the intention was here, but in Web2 most of the events being sent are singletons, so adding this 50msec delay just artificailly slows down the UI/UX for no benefit at all.

3 Likes