adobe-photoshop / spaces-design

Adobe Photoshop Design Space
http://adobe-photoshop.github.io/
Other
852 stars 74 forks source link

Fast history initialization for fresh documents #3723

Closed iwehrman closed 8 years ago

iwehrman commented 8 years ago

When creating a new document or opening a fresh document (i.e., the document was not previously open at application-load time), there is no need to query Photoshop for history state information because we already know that the the answer will be trivial: there is one state, and we're at that state. Since querying history can also be very slow (see #3706), we should skip it when we can. That's what this PR does, with an alternative to queryCurrentHistory called initNewHistory, which initializes the history store without first consulting Photoshop.

This PR is currently review-only because, mysteriously, this does not seem to result in a measurable speedup when document.opening a large document. I can't figure out why, and I suspect a quirk of the adapter or Photoshop. I'm posting this in case either a) it is obvious to someone else, from looking at the diff, why this is not faster, or b) so that I can point to this when @shaoshing and I meet with @jsbache and @jfitz-adobe next week for a JS-native profiling exercise.

iwehrman commented 8 years ago

Example timeline on master: image

Example timeline in this branch: image

shaoshing commented 8 years ago

I also did an experiment to temperately disable the queryCurrentHistory during startup and did not observe any improvement either, so it is likely that the queryCurrentHistory action and the descriptor it runs are not the cause. The reason it takes long time to complete is may be because PS is occupied by other running descriptors. If you run DS in my branch https://github.com/adobe-photoshop/spaces-design/tree/shao/console, turn on the Debug > Log Descriptor Events option, and open a large document, you will see the following:

screen shot 2016-02-19 at 9 29 00 am

548 is the history descriptor, and notice there are several other descriptors are executed asynchronously at the same time. Although 548 took 766ms to finish, the 3 descriptors ahead of it have the similar completion time (545, 546, 547), which means 1) they were fast 2) they were blocked by other descriptors. 542 543 are the descriptors to fetch document properties, their completion time are 491ms and 291ms (710 - 491), and they might be the descriptors that blocking the others.

shaoshing commented 8 years ago

537 is suspicious. It was the only running descriptor and took 736ms to return the document ID.

screen shot 2016-02-19 at 9 51 05 am
iwehrman commented 8 years ago

As @shaoshing and I discussed offline, we probably will just close this PR assuming we conclude with certainty that the historyState descriptor is not the culprit, given that this would adding a moderate amount of code complexity for little or no gain. But, we're going to leave this open a little while longer to let that research conclude.