Skip to content
This repository has been archived by the owner on Sep 6, 2021. It is now read-only.

Performance tests may not be capturing file open time in brackets-shell #1687

Open
peterflynn opened this issue Sep 18, 2012 · 9 comments
Open
Assignees

Comments

@peterflynn
Copy link
Member

Looking at our file-open performance data:
https://docs.google.com/spreadsheet/ccc?key=0Aras0diokeHxdEc5RGtOeVI0V0xGU3FPUXBuX3ZYTlE#gid=5

There was a dramatic drop in the recorded time to open large files when we cut over to brackets-shell. There's now almost no difference between different file sizes.

That seems a little suspicious given that most disk operations have seemed slightly slower in -shell. I'm wondering if the greater degree of asynchronicity could be exposing a bug in where we've placed the instrumentation. Seems worth investigating.

@ghost ghost assigned jasonsanjose Sep 18, 2012
@pthiess
Copy link
Contributor

pthiess commented Sep 18, 2012

Hi @peterflynn I would eventually consider this a medium priority, we need to trust the performance measurements. What's your take?

@peterflynn
Copy link
Member Author

@pthiess: sure, medium seems fair. There's also a slight chance this is a symptom of a functional async bug too, so it definitely seems worth investigating.

However, IMHO we shouldn't really trust any of these performance measurements :-) Because they capture so little of the rendering/layout cost, which can be substantial. I think the camera tests suggested that these file-open measurements inaccurate by 10-30%, depending on file size.

@pthiess
Copy link
Contributor

pthiess commented Sep 19, 2012

Agreed, we probably have no handle on being accurate about the layout cost, maybe using a set of files to calculate a compound average could minimize the experimental variance.

@pthiess
Copy link
Contributor

pthiess commented Sep 19, 2012

@peterflynn @jasonsanjose Raised priority to medium.

@pthiess
Copy link
Contributor

pthiess commented Sep 26, 2012

Reviewed

@jasonsanjose
Copy link
Member

I'm not seeing any functional issues in the perf instrumentation as-is. I traced through and debugged the call stack to be sure that we do add the editor to the DOM synchronously before we stop our perf timer. I didn't notice the disk slowdown that @peterflynn mentioned when we moved to CEF3. Is there something else that I'm missing?

@pthiess
Copy link
Contributor

pthiess commented Oct 22, 2012

@jasonsanjose Maybe do a perceptional testing using an older build with brackets-app vs. the current.

@lkcampbell
Copy link
Contributor

@jasonsanjose and @pthiess, this issue is really stale and doesn't seem like it is medium priority anymore. Do we ever reassign priorities based on the age of an issue? It seems like a really old medium priority issue should eventually turn into either a low priority issue or a high priority issue.

@pthiess
Copy link
Contributor

pthiess commented Jan 6, 2014

@lkcampbell Great question - I frequently ask the team to review their medium priority bugs. @jasonsanjose I wonder if we should just close this issue for now.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants