OfficeDev / office-js

A repo and NPM package for Office.js, corresponding to a copy of what gets published to the official "evergreen" Office.js CDN, at https://appsforoffice.microsoft.com/lib/1/hosted/office.js.
https://learn.microsoft.com/javascript/api/overview
Other
674 stars 95 forks source link

Microsoft 365 Version 2108 On windows 10 has made range value context.sync super slow #2154

Closed nsakar closed 1 year ago

nsakar commented 3 years ago

A range array created in typescript [proxy obects] takes very long to synchronize on excel sheet with upgrade to version 2108. This was working fine with version 2102 and shared runtime. have edge and webview2 version 94.0.992.31. The excell addin is virtually unusable on this office version. Please help urgently. The addins are in production and working great with office 2102.

Expected Behavior

We expect the behavior to remain the same as in version 2102. What changed?

Current Behavior

Super slow sync of proxy objects in range with excel sheet. Mal functioning code is attached below The delay is happening when context.sync is executed.

Steps to Reproduce, or Live Example

Context

Your Environment

Useful logs

rm2631 commented 3 years ago

We've been debugging for this for 2 days. Thanks.

erharvey commented 3 years ago

Same issue on our side. The plugin works perfectly on 2105 put becomes painfully slow with 2108 and 2109. Setting range values even without using untrack is now slow for a 120x200 range. We see the cursor reloading as if it is not a single batch command anymore but many small requests (As if we were looping instead of batching the set values commands).

We are using the following code before a context sync

bodyRange.values = entitiesProjectionArray; // entitiesProjectionArray is a 2d array

It is also urgent for us.

Thanks.

nsakar commented 3 years ago

In addition Grouping api does not function. No rows are displayed: We need an immediate fix public async GroupRows(workSheetName: string, map: Map<string, RowGroupObject>) {

    await Excel.run({ delayForCellEdit: true }, async function (context) {

        var sheet = context.workbook.worksheets.getItem(workSheetName);

        map.forEach((val, key) => {
            // iterate through the map to create row groupings
            const range = `${val.startRow}:${val.endRow}`;
            sheet.getRange(range).untrack().group(Excel.GroupOption.byRows);              
        });

        return await context.sync();

    }).catch(function (error) {

        //Need to create logger 
        console.log("Error: " + error);
        if (error instanceof OfficeExtension.Error) {
            console.log("Debug info: " + JSON.stringify(error.debugInfo));
        }

    });
}
honxmsft commented 3 years ago

We find a suspicious issue causing the performance issue, which drops about 90% perf. The fix of it will be checked soon. I'll update the thread after the change is checked-in.

@nsakar Can you provide a sample code for the group issue? I've tried such code but it works fine.


interface RowGroupObject {
  startRow: string;
  endRow: string;
}

async function run() {
  await Excel.run(async (context) => {
    const map = new Map<string, RowGroupObject>();
    map.set("1", { startRow: "1", endRow: "3" });
    map.set("2", { startRow: "5", endRow: "10" });
    map.set("3", { startRow: "13", endRow: "16" });
    map.set("4", { startRow: "18", endRow: "20" });
    GroupRows("Sheet2", map);
  });
}
razor1299 commented 3 years ago

Same issue. The plugin works perfectly but big performance issue which drops with the version Excel 2108. I try different format string, int, with formula without formula etc. it's an urgent problem. Thank you.

gingerjia commented 3 years ago

@razor1299 @nsakar @rm2631 @erharvey sorry for the impact! We are working hard to deploy the fix for the suspicious causing the perf regression. At same time, we also try to mitigate the issue by turn off the regression code. Can you help to double confirm the issue is disappearing by end of tomorrow? If you still hit the perf issue, please help to let me know your build number then I do further confirmation. Thanks! Really sorry for the issue and thanks very much for your coordination and reporting the issue!

erharvey commented 3 years ago

Hi @gingerjia , yes we will try it by tomorrow.

nsakar commented 3 years ago

We find a suspicious issue causing the performance issue, which drops about 90% perf. The fix of it will be checked soon. I'll update the thread after the change is checked-in.

@nsakar Can you provide a sample code for the group issue? I've tried such code but it works fine.

interface RowGroupObject {
  startRow: string;
  endRow: string;
}

async function run() {
  await Excel.run(async (context) => {
    const map = new Map<string, RowGroupObject>();
    map.set("1", { startRow: "1", endRow: "3" });
    map.set("2", { startRow: "5", endRow: "10" });
    map.set("3", { startRow: "13", endRow: "16" });
    map.set("4", { startRow: "18", endRow: "20" });
    GroupRows("Sheet2", map);
  });
}

I already attached in comments below but providing again: public async GroupRows(workSheetName: string, map: Map<string, RowGroupObject>) {

    await Excel.run({ delayForCellEdit: true }, async function (context) {

        var sheet = context.workbook.worksheets.getItem(workSheetName);

        map.forEach((val, key) => {
            // iterate through the map to create row groupings
            const range = `${val.startRow}:${val.endRow}`;
            sheet.getRange(range).untrack().group(Excel.GroupOption.byRows);              
        });

        return await context.sync();

    }).catch(function (error) {

        //Need to create logger 
        console.log("Error: " + error);
        if (error instanceof OfficeExtension.Error) {
            console.log("Debug info: " + JSON.stringify(error.debugInfo));
        }

    });
}
erharvey commented 3 years ago

We tested this morning with both 2105 and 2109 and it seems to work fine now.

Thanks

rm2631 commented 3 years ago

Can confirm also. It is solved. Thank you.

razor1299 commented 3 years ago

Problem solved. Thank you.

gingerjia commented 3 years ago

@erharvey, @rm2631 and @razor1299 thanks very much for helping confirm the fix! I will close this thread. If have any issues or concerns, please feel free to let us know!

gingerjia commented 3 years ago

@erharvey, @rm2631 and @razor1299 thanks very much for helping confirm the fix! If have any issues in future, please feel free to let us know!

nsakar commented 3 years ago

Guys

Although issue is fixed in current channel 2108,2109 however it needs to be fixed in teh semi annual enterprise channel where it is still an issue

Semi-Annual Enterprise. This is the version that enterprises use to deploy across the board. Please help

nsakar commented 3 years ago

@erharvey, @rm2631 and @razor1299 thanks very much for helping confirm the fix! I will close this thread. If have any issues or concerns, please feel free to let us know!

Please fix this in Semi Annual Enterprise channel also where it is still an issue

gingerjia commented 3 years ago

Guys

Although issue is fixed in current channel 2108,2109 however it needs to be fixed in teh semi annual enterprise channel where it is still an issue

Semi-Annual Enterprise. This is the version that enterprises use to deploy across the board. Please help

Working on it. Thanks @nsakar ! will update the thread when have progress!

gingerjia commented 3 years ago

@nsakar our fix for SAC Preview Channel is check-in today, and when get deployed, I will update here. Another thing I want to double confirm is that do you mean you get the issue at SAC? Or SAC Preview channel. From our side expectation, the issue should only impact SAC Preview, but not impact SAC. Thanks very much for your help!

gingerjia commented 2 years ago

@nsakar For SAC Preview Channel, we have deployed the fix and validated it works as expected. Please let me know if you have any further issues. Thanks!

donlvMSFT commented 1 year ago

I'd like to close this thread as the issues been resolved. Thanks for discussion!