Blockly: domToText: Performance issue in large projects

[eanders-MS]

I'm investigating performance issues when working with large blocks programs (in MakeCode Arcade), and Blockly's domToText method came up as a hot path due to a regexp replace it executes. This regexp was added in https://github.com/google/blockly/pull/2663 (Textarea Field, Multiline Block (from acbart)). Here's the line in question: https://github.com/google/blockly/blob/ba68081d8f0f8e81d76baf370716d5955bae82a1/core/xml.js#L319

This regexp runs repeatedly on the document, causing domToText to take a significant amount of time when the xml is very large. Without this regexp, saving takes practically no time at all.

If you could help me understand the purpose of this search/replace, perhaps I could come up with another approach. And if you have a suggestion for how to improve or remove it, I would be happy to try it and report back.

OS: Windows 10
Browser: Chrome 83

[BeksOmega]

Hi! I'm not actually on the Blockly team, but I do know some stuff about this area, and I think I have some ideas =)

So it looks like this was added by the same change that added the multiline input field. It looks like it's going through and replacing all \n (linebreak) characters with the escaped versions so that the field correctly round-trips through XML.

Example escaped xml:

<xml xmlns="https://developers.google.com/blockly/xml">
  <block type="test_fields_multilinetext" id="jGDBz^O9+Zz6-r@84/|u" x="13" y="13">
    <field name="CODE">default1&#10;default2</field> <!-- See &#10; escaping -->
  </block>
</xml>

So it is necessary for the multiline text field, but I don't think it's necessary to run the search-and-replace over /all/ of the XML.

If you want to take a crack at fixing this I would move the search-and-replace into a toXml function that lives on the multiline text field. That should (hopefully) save time in serialization because the search-and-replace will only run when it absolutely has to.

Thank you so much for looking into this that's really cool!

[eanders-MS]

@BeksOmega thank you that info is super helpful! I made a fix in MakeCode's fork: https://github.com/microsoft/pxt-blockly/pull/315, and will open one here too.

[rachel-fenichel]

Good catch! Yes, we definitely want that PR.

[eanders-MS]

Working on this now, but want to test it and having trouble getting the playground to run locally. Loading playground.html from file protocol, getting Uncaught ReferenceError: goog is not defined errors in the dev console.

[rachel-fenichel]

Can you try rebuilding (npm run build) and reloading the playground?

[eanders-MS]

I did try that. It seems to build fine. Does this output look correct?

> [email protected] build C:\code\google\blockly
> gulp build

[09:44:59] Using gulpfile C:\code\google\blockly\gulpfile.js
[09:44:59] Starting 'build'...
[09:44:59] Starting 'buildLangfiles'...
[09:44:59] Starting 'buildCompressed'...
[09:44:59] Starting 'buildBlocks'...
[09:44:59] Starting 'buildUncompressed'...
[09:44:59] Starting 'buildJavascript'...
[09:44:59] Starting 'buildPython'...
[09:44:59] Starting 'buildPHP'...
[09:44:59] Starting 'buildLua'...
[09:44:59] Starting 'buildDart'...
Created file: .\msg\json\en.json
Created file: .\msg\json\qqq.json
[09:45:00] Finished 'buildLangfiles' after 835 ms
[09:45:02] Finished 'buildPython' after 2.6 s
[09:45:02] Finished 'buildJavascript' after 2.61 s
[09:45:02] Finished 'buildLua' after 2.61 s
[09:45:02] Finished 'buildDart' after 2.62 s
[09:45:02] Finished 'buildPHP' after 2.62 s
[09:45:02] Finished 'buildBlocks' after 2.64 s
[09:45:04] Finished 'buildUncompressed' after 4.27 s
[09:45:06] Finished 'buildCompressed' after 7.05 s
[09:45:06] Finished 'build' after 7.05 s

[samelhusseini]

That all looks right.

Couple of things to confirm:

• You have checked out google/blockly
• You are working off the develop branch
• You are loading the playground in the browser navigating to file:///BLOCKLYROOT/tests/playground.html

If the above all looks good to you, I would try stashing any changes to make sure that no local changes are causing the error.

[eanders-MS]

After stashing local changes it works! Could I be missing a build flag?

[samelhusseini]

It's unclear to me how your local changes may have affected the playground loading (I would have to see those), but in general there should only be a need to run the build for the playground (uncompressed) if you've added new files.
The rest of the build scripts build other binaries such as:

• The production build of core Blockly (compressed)
• Blockly blocks (production)
• Blockly language generators (production)

We generally refrain from checking these in (as they are binaries), and usually only once per release.

There are other advantages to running the build though, specifically for the core library, as it allows us to run the closure compiler that compiles the library and notifies us of any issues (usually type issues).

This also runs on travis, so the travis build will complain if you don't happen to run that before PR'ing against google/blockly.

[eanders-MS]

Oh I didn't realize I didn't need to rebuild the uncompressed. If that isn't necessary for testing my changes, then I will avoid it. Thanks for your help.