search

contentsync / SketchContentSync

Sync sketch files with google docs.
275 stars 17 forks source link

Pull gets slower with increasing number of pages #131

Closed nilsdoehring closed 3 years ago

nilsdoehring commented 3 years ago

My real-world project has 60 pages containing a mix of the scenarios described below. A pull takes 50 seconds, while the example from Getting Started takes 3 to 5 seconds.

Here are steps to reproduce in v8.3.1:

Part 1

Result: This pull takes 10 to 14 seconds. A pull in the Sketch file from Getting Started takes 3 to 5 seconds. (Results may vary by machine and connection).

Part 2

The time a pull will take can be further increased by duplicating main-flow:

var sketch = require('sketch')

var document = sketch.getSelectedDocument()
var page = document.pages.find(page=>page.name == "main-flow");
var newPage;

for(var i = 0; i<50; i++){
  newPage = page.duplicate();
  newPage.name = page.name + " " + i;
}

Part 3

The time a pull will take can be extended to several minutes:

While I like coffee, this doesn't feel like expected behaviour.

dbrody commented 3 years ago

@nilsdoehring Thank you for the detailed submission. Can you please describe the shape of your spreadsheet as well? How many keys / sheets are on your spreadsheet during each of these parts?

nilsdoehring commented 3 years ago

@dbrody Sure! The spreadsheet is a single sheet with 60 versions (columns) at 26 keys (rows) each. Two of these keys are symbol type IDs, the remaining 24 are just plain text, no style. I can share if needed, but as stated in the description, the issue can be reproduced based on the simple sheet from the Getting Started example.

dbrody commented 3 years ago

@nilsdoehring Another thing, can you provide the machine / OS / Sketch / ContentSync versions you are on? We are doing some testing and want to make sure we can compare accurately.

nilsdoehring commented 3 years ago

@dbrody sure:

dbrody commented 3 years ago

@nilsdoehring Thank you. We have benchmarked the same behavior and are working on a few solutions.

dbrody commented 3 years ago

@nilsdoehring We are working to address this issue.

An initial improvement has been released in our 8.4.0-alpha release. Please note that it is an alpha release so it has not gone under heavy stress testing. We'd love feedback as we work towards a production release of these improvements.

nilsdoehring commented 3 years ago

@dbrody I went through the scenarios described above, here's the updated benchmark:

This is awesome!

Some issues I've noticed:

objc[24626]: Class AFHTTPSessionManager is implemented in both /Users/ndoehring/Library/Application Support/com.bohemiancoding.sketch3/Plugins/SketchContentSync.sketchplugin/Contents/Sketch/frameworks/ContentSync.framework/ContentSync (0x155d6bbf0) and /Users/ndoehring/Library/Application Support/com.bohemiancoding.sketch3/Plugins/SketchContentSync.sketchplugin/Contents/Sketch/frameworks/ContentSyncRealtime.framework/ContentSyncRealtime (0x1888991a8). One of the two will be used. Which one is undefined. [... several more of these ...]

dbrody commented 3 years ago

@nilsdoehring Thanks for the feedback.

  1. Pull Full Design will update the full design file, not just the current page. That is expected behavior. That number is the total number of items being synced I believe. So that includes every override etc as well in the count. This is expected to take some time since that is a large update. And while we are doing our best to avoid hanging moments, there are small moments where we need to hang the main UI to update the sketch layers on large updates.
  2. We are still fine tuning the performance and collisions. Any crash reports you can send to our support@contentsync.com would be helpful. Also, the update does allow for much larger syncs. Before we limited it to up to 1000 updates. However the new performance increases allow us to remove that constraint. So a full design pull will now update the entire file. Before if the file was too big, it would limit to the current page.
  3. Do you see that error if you restart Sketch? Making sure you only have one copy of the plugin installed?
nilsdoehring commented 3 years ago
  1. Thanks for the explanation – prior to 8.4.0-alpha, I was probably experiencing the limitation that you've mentioned in 2.
  2. Will do / Updating an entire design file is not part of my use cases, but good to know!
  3. Indeed, I am not able to reproduce this. Must have been Sketch being weird with the plugin update.
dbrody commented 3 years ago

@nilsdoehring New update with 8.4.1-alpha improves this even more. It slows down the syncing a little but should prevent the crashing you were experiencing. Let me know how that works for you and we can close this issue.

nilsdoehring commented 3 years ago

@dbrody this is perfect, thank you so much!