Change behavior of pending event triggers

[dawoodkhan82]

Description

Change default behavior of event triggers to "wait" for pending requests to complete. Fixes issues related textbox change events not registering latest change. ex: #1954

TODO: make this work for queue

Please include:

• relevant motivation
• a summary of the change
• which issue is fixed.
• any additional dependencies that are required for this change.

Closes: #1954

Checklist:

• I have performed a self-review of my own code
• I have added a short summary of my change to the CHANGELOG.md
• My code follows the style guidelines of this project
• I have commented my code in hard-to-understand areas
• I have made corresponding changes to the documentation
• I have added tests that prove my fix is effective or that my feature works
• New and existing unit tests pass locally with my changes

A note about the CHANGELOG

Hello 👋 and thank you for contributing to Gradio!

All pull requests must update the change log located in CHANGELOG.md, unless the pull request is labeled with the "no-changelog-update" label.

Please add a brief summary of the change to the Upcoming Release > Full Changelog section of the CHANGELOG.md file and include
a link to the PR (formatted in markdown) and a link to your github profile (if you like). For example, "* Added a cool new feature by [@myusername](link-to-your-github-profile) in [PR 11111](https://github.com/gradio-app/gradio/pull/11111)".

If you would like to elaborate on your change further, feel free to include a longer explanation in the other sections.
If you would like an image/gif/video showcasing your feature, it may be best to edit the CHANGELOG file using the
GitHub web UI since that lets you upload files directly via drag-and-drop.

[github-actions]

All the demos for this PR have been deployed at https://huggingface.co/spaces/gradio-pr-deploys/pr-2436-all-demos

[abidlabs]

Very interesting, some questions I have:

• What does "existing pending request" mean? Does it mean when there is (A) any request that the backend is processing, (B) any request from the same user that the backend is processing, or (C) any request from the same user and same event that the backend is processing
• What will be the default behavior_when_pending? I think most users will expect the default to be "wait" although the current behavior matches what "block". I'm not sure when you'd want the other two -- can you explain the use cases for those options?
• How will this work with live=True? Will users pass in in behavior_when_pending to Interface as well?

[pngwn]

I don't think we want both behaviours, and I don't really like the idea of adding more API surface area to solve this. I think we should just honour the latest request at any given moment (letting them all through), rather than rejecting subsequent request until we have a response.

• req 1
• req 2
• req 3
• res 1 (ignore)
• res 2 (ignore)
• res 3 (handle)

Now it would also be nice if we could 'abort' those processes on the backend where possible too but depending on the stage they are at that might not be possible. If the queue is active we could remove them from the queue though.

You can't really abort requests in the browser but we could signal to the server that it should ignore them if they are not already being processed. I think we'll need to assign ids or hashes to requests though because, especially for the queue otherwise we won't know what response we are dealing with.

[aliabid94]

I think there are clear cases where we want each of these behaviors:

• block: this is the current default behavior. In an interface with a heavy model, you usually don't want the user to be able to click Submit while the previous request is pending.
• run: Users may wish to allow multiple requests to be triggered. You may wish to spin off multiple backend processes without waiting for the previous process to finish.
• wait: this is very useful for "change" event listeners (the original reason for this PR). If a textbox has a change event listener, you don't want to fire on every single letter if the last pending request has not completed (as would happen in run mode). But block mode is not appropriate either: if a user types letter "A" (which fires a request), then types two more letters "BC" while the request is pending, those letters will not trigger a request. Later after the request completes, the user types "D", then this will trigger a change with the value "ABCD". However, we never triggered on "ABC" when there was no request pending. With wait, after the first request completes, it sees the value has changed since the request was first fired would have fired again with "ABC".

I definitely can see a user wanting to be able to use each of these three behaviors.

[abidlabs]

I also think we avoid increasing the API. Instead of having our users think through these options, what about if we were to simply change the default behavior to "wait"? Wouldn't that solve all of the issues?

• If a user has a heavy model, then if they click submit a bunch of times, only the first request and final request will go through (assuming that the model is heavy so it will be running during the intermediate requests)
• Clearly, it will solve the original issue When multiple events are triggered consecutively, the final event should always be sent #1954
• I don't think the use case for run is particularly common and it could clog up the queue so we shouldn't support it

[pngwn]

We could just batch requests per client.

• send request (gets processed)
• request
• request
• first request completes and is returned
• subsequent requests are handled as a single step (either they're collected and batched or one is chosen and processed)

It works for heavy models: you'll never run another while one is running. And if there's is a large queue of requests you still only run one. This assumes they are idempotent which they might not be due to state, they could be batched.

For live text inputs, same story really, we discard the intermediate requests.

I'm still not sure what the run usecase is really, since the output would be a single component they would just overwrite each other, with multiple threads are there even any guarantees of order? I guess if you were triggering a bunch of side effects or logging stuff the then you would be happy to just launch them whenever, some of these kinds of requests could probably be batched though. Would need to understand the concrete usecases for this.

[abidlabs]

@pngwn by batching, are you referring to collecting a list of samples and passing them into the prediction function as a list (as in this PR that I've been working on)? This requires the function to accept batched inputs, so it wouldn't always work.

But basically as long as:

we discard the intermediate requests.

we should be fine!

[pngwn]

Not really. I guess just discarding intermediate requests is fine but this won't work when we have state. If those intermediate requests make use of gr.State then the results will possibly be wrong in confusing ways if we don't batch those state updates somehow. But that might be difficult.

[aliabid94]

I actually feel that all three modes will have very valid use-cases as people build more complex interfaces, and I don't think that this is significantly expanding the API surface. It's also possible to disallow "run" mode if any of the inputs or outputs are state.
I'm okay to move to "wait" behavior only for all event listeners for now, but will not be surprised if a user later asks to be able to launch multiple processes by clicking a button multiple times.

[abidlabs]

@dawoodkhan82 this might close #1453 as well?

[abidlabs]

I synced with @dawoodkhan82 on this a bit, and here's what we're thinking:

• Let's make the default behavior to be wait and leave everything else to avoid unnecessary complexity for now. If there are strong use cases for other behaviors then we can consider adding them, but for now they are too "low-level".

cc @freddyaboulton @pngwn @aliabid94

[pngwn]

This is technically a breaking change. Would we consider this a bug fix instead? Have we tested to ensure this doesn't break any other existing use-cases or cause issues because of the change in behaviour?

[abidlabs]

Good point @pngwn. Here's a demo that would "no longer work" as expected:

import gradio as gr
import time

def slow_identity(x):
    time.sleep(1)
    return x

with gr.Blocks() as demo:
    
    with gr.Row():
        i1 = gr.Textbox()
        o1 = gr.Textbox()
    with gr.Row():
        i2 = gr.Textbox()
        o2 = gr.Textbox()
    with gr.Row():
        i3 = gr.Textbox()
        o3 = gr.Textbox()        
    i1.change(slow_identity, i1, o1)
    i2.change(slow_identity, i2, o2)
    i3.change(slow_identity, i3, o3)  
    
demo.launch()

Currently, if you make changes really quickly to i1, i2, and i3 in sequence, they will all show up in o1, o2, o3. But with this PR, the changes in i2 will not show up

I still think that having the "wait" behavior is overall better, but perhaps we should make this change in 4.0 to avoid any breaking changes we don't foresee. WDYT about kicking this PR and associated issues to 4.0 milestone?

[pngwn]

Closing for now.