Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Bug] Async function handling behavior #4648

Open
2 tasks done
jcalem opened this issue Feb 14, 2025 · 2 comments
Open
2 tasks done

[Bug] Async function handling behavior #4648

jcalem opened this issue Feb 14, 2025 · 2 comments
Assignees
Labels
bug Something isn't working scope:formula

Comments

@jcalem
Copy link
Contributor

jcalem commented Feb 14, 2025

Before you submit this issue, have you checked the following

  • Is this really a problem?
  • I have searched the Github Issues for similar issues, but did not find anything.

Affected packages and versions

0.6.0

Reproduction link

Followed this guide:
https://docs.univer.ai/en-US/guides/sheets/advanced/custom-formula

Steps to reproduce:

  • Implement an async custom function
  • See below screen recordings

Expected behavior

Issue 1: Async function nodes should be processed in parallel

Issue 2: Async functions triggers should not overwrite each other

I'm working on a project that involves creating custom functions that perform HTTP calls to backend service to retrieve values. Async function handling is essential for this.

Actual behavior

Issue 1: Formula engine calculates async nodes sequentially

Description: The function interpreter awaits async nodes for each child node and seems to execute sequentially, rather than in parallel. The screen recording below shows a fill operation that takes over 15+ secs (each CUSTOM_ASYNC_OBJECT awaits for 3 seconds) due to this behavior.

Screen.Recording.2025-02-14.at.3.27.22.PM.mov

Issue 2

Description: When more than two async nodes are triggered separately, only one of the values is executed and updated in the sheet. In the following screen recording, the Test 6 calculation was overwritten due to the overlapping async calls.

Screen.Recording.2025-02-14.at.3.25.06.PM.mov

System information

  System:
    OS: macOS 14.4.1
    CPU: (10) arm64 Apple M1 Pro
    Memory: 80.33 MB / 16.00 GB
    Shell: 5.9 - /bin/zsh
  Binaries:
    Node: 20.11.1 - ~/.nvm/versions/node/v20.11.1/bin/node
    Yarn: 1.22.19 - /opt/homebrew/bin/yarn
    npm: 10.5.0 - ~/.nvm/versions/node/v20.11.1/bin/npm
  Browsers:
    Chrome: 133.0.6943.98
    Safari: 17.4.1
@univer-bot univer-bot bot added the bug Something isn't working label Feb 14, 2025
@jcalem
Copy link
Contributor Author

jcalem commented Feb 19, 2025

@Dushusir Any thoughts on how to fix this?

@jcalem
Copy link
Contributor Author

jcalem commented Feb 24, 2025

This issue is caused due to the fundamental architecture/configuration of the CalculateFormulaService, Interpreter. These classes rely on the following runtimeService reference data during the calculation execution:

  • row: number,
  • column: number,
  • rowCount: number,
  • columnCount: number,
  • sheetId: string,
  • unitId: string

Here is a snippet from CalculateFormulaService:

        const treeCount = treeList.length;
        for (let i = 0; i < treeCount; i++) {
            const tree = treeList[i];
            const nodeData = tree.nodeData;
            const getDirtyData = tree.getDirtyData;
            ...
            this._runtimeService.setCurrent(
                tree.row,
                tree.column,
                tree.rowCount,
                tree.columnCount,
                tree.subUnitId,
                tree.unitId
            );
            let value: FunctionVariantType;
            if (getDirtyData != null && tree.featureId != null) {
                ....
            } else if (nodeData != null) {
                if (interpreter.checkAsyncNode(nodeData.node)) {
                    value = await interpreter.executeAsync(nodeData);
                } else {
                    value = interpreter.execute(nodeData);
                }
            ...

When a cell update is made, the dependency tree is re-calculated and executed sequentially (in-reverse) using the above code block. The current implementation uses an 'await', performing the calculation sequentially rather than in parallel. The interpreter and function executor rely on the current _runtimeService's reference data. So if multiple functions are reading/writing to the same variables, the values will be overwritten and the calculation result will be incorrect.

A significant amount of changes will be required in order to correct this to process the dependency tree in parallel. However, this is a crucial feature for any application that requires integration with custom functions. Based on this, is it possible to prioritize this issue?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working scope:formula
Projects
None yet
Development

No branches or pull requests

3 participants