Feature Request: Reducing frames on handling promise

[FurryR]

Feature

In current implementation, we usually need at least 2 frames to handle a promise (that is returning from blockFunction()):

Frame 1 -> (thread resumes)
        -> arg blockFunction()
        -> handlePromise()
        -> primitiveReportedValue.then()
Yield   -> (primitiveReportedValue handler)
Frame 2 -> stepThread()
        -> (thread resumes)
        -> op blockFunction()
        -> ...

However if we resume the thread on then handler, it will cost only 1 frame to do so:

Frame 1 -> (thread resumes)
        -> arg blockFunction()
        -> handlePromise()
        -> primitiveReportedValue.then()
Yield   -> (primitiveReportedValue handler)
        -> (thread resumes immediately)
        -> op blockFunction()
        -> ...
Frame 2 -> (reduced)

It is possible by stepping the thread immediately on handlePromise():

in src/engine/execute.js::handlePromise, L107 (see NOTE line)

const handlePromise = (primitiveReportedValue, sequencer, thread, blockCached, lastOperation) => {
    if (thread.status === Thread.STATUS_RUNNING) {
        // Primitive returned a promise; automatically yield thread.
        thread.status = Thread.STATUS_PROMISE_WAIT;
    }
    // Promise handlers
    primitiveReportedValue.then(resolvedValue => {
        handleReport(resolvedValue, sequencer, thread, blockCached, lastOperation);
        // If it's a command block or a top level reporter in a stackClick.
        if (lastOperation) {
            let stackFrame;
            let nextBlockId;
            do {
                // In the case that the promise is the last block in the current thread stack
                // We need to pop out repeatedly until we find the next block.
                const popped = thread.popStack();
                if (popped === null) {
                    return;
                }
                nextBlockId = thread.target.blocks.getNextBlock(popped);
                if (nextBlockId !== null) {
                    // A next block exists so break out this loop
                    break;
                }
                // Investigate the next block and if not in a loop,
                // then repeat and pop the next item off the stack frame
                stackFrame = thread.peekStackFrame();
            } while (stackFrame !== null && !stackFrame.isLoop);

            thread.pushStack(nextBlockId);
        }
        // NOTE: TODO: step thread immediately here, just like `startHats` do.
    }, rejectionReason => {
        // Promise rejected: the primitive had some error.
        // Log it and proceed.
        log.warn('Primitive rejected promise: ', rejectionReason);
        thread.status = Thread.STATUS_RUNNING;
        thread.popStack();
        // NOTE: TODO: step thread immediately here, just like `startHats` do.
    });
};

[FurryR]

I also noticed that there is an error in execute function:

in src/engine/execute.js::handlePromise, L516

        if (isPromise(primitiveReportedValue)) {
            handlePromise(primitiveReportedValue, sequencer, thread, opCached, lastOperation);

            // Store the already reported values. They will be thawed into the
            // future versions of the same operations by block id. The reporting
            // operation if it is promise waiting will set its parent value at
            // that time.
            thread.justReported = null;
            currentStackFrame.reporting = ops[i].id;
            currentStackFrame.reported = ops.slice(0, i).map(reportedCached => {
                const inputName = reportedCached._parentKey;
                const reportedValues = reportedCached._parentValues;

                if (inputName === 'BROADCAST_INPUT') {
                    return {
                        opCached: reportedCached.id,
                        inputValue: reportedValues[inputName].BROADCAST_OPTION.name
                    };
                }
                return {
                    opCached: reportedCached.id,
                    inputValue: reportedValues[inputName]
                };
            });

            // We are waiting for a promise. Stop running this set of operations
            // and continue them later after thawing the reported values.
            break;
        }

The result will be discarded if callback is already executed before thread.justReported = null;.

[adroitwhiz]

I believe this is intentional. The threads are currently stepped in a specific order, and I'm not sure about the side effects of stepping threads "out of order". Even though promises can resolve at any point, their threads will be stepped in the original execution order.

[adroitwhiz]

In fact, one of my refactorings is to move all control flow handling out of the promise handler so that only Sequencer#stepThreads does so.

[FurryR]

I believe this is intentional. The threads are currently stepped in a specific order, and I'm not sure about the side effects of stepping threads "out of order". Even though promises can resolve at any point, their threads will be stepped in the original execution order.

so I made some experiments about executing the thread "out of order" (see https://github.com/FurryR/lpp-scratch/blob/main/src/impl/thread/helper.ts#L34) like synchronous one. It works pretty fine. Seems that everything will be fine if you just restore util.thread after stepping the thread.

My point is, if an extension returns a synchronous value, it resumes immediately instead of yielding. Stepping the thread immediately after promise resolving makes blocks returning promise acts like synchronous and I think it is less confused. Thread execution order does not matter since Scratch users aren't required to learn about that, and few projects (FPS detection based on Promise and Turbowarp compiler detection based on reporter side-effect, both of them are cursed) are dependent on it.