File Open Performance Tests #841

tvoliter · 2012-05-08T22:03:33Z

Added instrumentation to functions relating to file open
Added Performance-tests.js Jasmine spec so perf tests can run in automation. Spec is currently disabled and not part of the unit test suite. Dev must manually enable it when doing perf testing.
perf test files are NOT part of repo since they are real world files. Files live in brackets-scenario
Added enabled/disabled state to PerfUtils
Made markStart() in PerfUtils return the timer name so it doesn't have to be created twice by clients
Added getDelimitedPerfData() to perfUtils and added area text in performance results Window with data to allow devs to easily store performance data.

jasonsanjose · 2012-05-08T22:44:06Z

reviewing

jasonsanjose · 2012-05-09T16:45:04Z

src/debug/DebugCommandHandlers.js

@@ -62,7 +62,8 @@ define(function (require, exports, module) {
    function _handleShowPerfData() {
        var $perfHeader = $("<div class='modal-header' />")
            .append("<a href='#' class='close'>&times;</a>")
-            .append("<h1 class='dialog-title'>Performance Data</h1>");
+            .append("<h1 class='dialog-title'>Performance Data</h1>")
+            .append("<div align=right>Raw data (copy paste out): <textarea rows=1 style='width:30px; height:8px; overflow: hidden; resize: none' id='brackets-perf-raw-data'>" + PerfUtils.getDelimitedPerfData() + "</textarea></div>");


I feel like I've seen ways to copy text from a web page, but all my googling results in a Flash-based solution.

I thought of that too and I found a number of ways to do it on stack overflow, but it wasn't simple so I went for the current solution since it's stop gap until we have performance reporting

Um, so I just tried the feature and I can't do CMD+A to select all. I have to drag a selection and CMD+C to copy out of the textarea. Is there anything better we can do?

I forgot that does not work on Mac but it does on PC. Let me try something else...

I fixed this by selecting all the text by a click handler on the field.

Ty

-----Original Message-----
From: Jason San Jose [mailto:reply@reply.github.com]
Sent: Thursday, May 10, 2012 11:14 AM
To: Ty Voliter
Subject: Re: [brackets] File Open Performance Tests (#841)

@@ -62,7 +62,8 @@ define(function (require, exports, module) {
function _handleShowPerfData() {
var $perfHeader = $("
")
.append("×")

.append("<h1 class='dialog-title'>Performance Data</h1>");

.append("<h1 class='dialog-title'>Performance Data</h1>")

.append("<div align=right>Raw data (copy paste out): <textarea rows=1 style='width:30px; height:8px; overflow: hidden; resize: none' id='brackets-perf-raw-data'>" + PerfUtils.getDelimitedPerfData() + "</textarea></div>");

Um, so I just tried the feature and I can't do CMD+A to select all. I have to drag a selection and CMD+C to copy out of the textarea. Is there anything better we can do?

Reply to this email directly or view it on GitHub:
https://github.com/adobe/brackets/pull/841/files#r803784

jasonsanjose · 2012-05-09T17:39:17Z

test/spec/Performance-test.js

+
+	describe("Performance Tests", function () {
+
+            	var testPath = SpecRunnerUtils.getTestPath("/../../../brackets-scenario/OpenFileTest/"),


The assumption for the repo location should be documented. Perhaps we can stuff this in a config file later. New issue?

Commented added. Ultimately I think we want test media that we can make public so that anyone can run the tests. I commented about this too

jasonsanjose · 2012-05-09T17:53:13Z

Initial review complete

jasonsanjose · 2012-05-10T17:58:55Z

need to merge with master

jasonsanjose · 2012-05-10T18:18:38Z

Second pass complete. Since this is off by default, and we don't have an automation task for this story, I think this is fine as-is with TODOs indicated.

tvoliter · 2012-05-10T23:47:29Z

Cool. Fixes submitted!

Ty

-----Original Message-----
From: Jason San Jose [mailto:reply@reply.github.com]
Sent: Thursday, May 10, 2012 11:19 AM
To: Ty Voliter
Subject: Re: [brackets] File Open Performance Tests (#841)

Second pass complete. Since this is off by default, and we don't have an automation task for this story, I think this is fine as-is with TODOs indicated.

Reply to this email directly or view it on GitHub:
#841 (comment)

jasonsanjose · 2012-05-11T16:16:47Z

src/project/FileViewController.js

@@ -87,6 +89,8 @@ define(function (require, exports, module) {
        }

        $(exports).triggerHandler("documentSelectionFocusChange");
+
+        PerfUtils.addMeasurement(perfTimerName);


jslint error. perfTimerName was used before it was defined.

…perf * origin/master: (110 commits) Add comment. Make sure cursor is visible after a change event is received. code review changes Add a title to the project title in the sidebar refactor perfutils to allow measurement to be updated. update TypingSpeedLogger to use new perfutils api. fix comment for _setWidth Cleanup sidebar resizer. Fix edge case when moving from snapped closed to open. Update CodeMirror SHA Update README.md fix merge issues Don't resize shadow when using absolute positioning. Fix scroller shadow resize Increase sidebar resizer width. Fix dblclick bugs. convert markStart to also accept an array Use updated hyphenated names Code review comments. Fixes selection marker when closing jstree nodes convert multiple value output to min/avg/max/last Update CodeMirror SHA Call ProjectManager.loadProject() after extensions have been loaded. fixed the dumb ol issue that I thought I fixed before with the indent. @tvoltier found it with his hawk eyes. ... Conflicts: src/editor/EditorManager.js src/utils/PerfUtils.js

jasonsanjose · 2012-05-11T19:41:35Z

src/editor/Editor.js

@@ -64,6 +64,7 @@ define(function (require, exports, module) {
    var EditorManager    = require("editor/EditorManager"),
        Commands         = require("command/Commands"),
        CommandManager   = require("command/CommandManager"),
+        PerfUtils           = require("utils/PerfUtils"),


Indent of assignment

Fixed and pushed

-----Original Message-----
From: Jason San Jose [mailto:reply@reply.github.com]
Sent: Friday, May 11, 2012 12:42 PM
To: Ty Voliter
Subject: Re: [brackets] File Open Performance Tests (#841)

@@ -64,6 +64,7 @@ define(function (require, exports, module) {
var EditorManager = require("editor/EditorManager"),
Commands = require("command/Commands"),
CommandManager = require("command/CommandManager"),

PerfUtils = require("utils/PerfUtils"),

Indent of assignment

Reply to this email directly or view it on GitHub:
https://github.com/adobe/brackets/pull/841/files#r810596

File Open Performance Tests

…nd menu font fixed to match filetree, fix 2

…nges : Fix 2

…ion in the right-click menu (adobe#841)

tvoliter added 5 commits May 8, 2012 11:53

Adding performance measuring code around file open functions.

c27b73a

Make perf data show up after test is run. Add raw data to perf window

e87ea62

Add perf tests to test specs

08b3332

Cleanup

3ed793c

Change order of data in getDelimitedPerfData

e105644

jasonsanjose reviewed May 9, 2012
View reviewed changes

ghost assigned jasonsanjose May 9, 2012

jasonsanjose reviewed May 9, 2012
View reviewed changes

tvoliter added 2 commits May 9, 2012 15:57

separating linting vs dom lint display

5360f60

code review fixes

76773ae

code review cleanup

4ae6fa8

jasonsanjose reviewed May 11, 2012
View reviewed changes

tvoliter added 3 commits May 11, 2012 10:34

lint cleanup

89679da

missed from last commit

f280622

tvoliter added 3 commits May 11, 2012 12:11

updated references to test media

ed5ee49

Improve markStart recursive warning message

9831173

Move call to markStart() so it doesn't occur when JSLint is disabled

6b29a3a

jasonsanjose reviewed May 11, 2012
View reviewed changes

fix indentation

f6d90a0

jasonsanjose added a commit that referenced this pull request May 11, 2012

Merge pull request #841 from adobe/tvoliter/open-file-perf

c4ff795

File Open Performance Tests

jasonsanjose merged commit c4ff795 into master May 11, 2012

Th30 referenced this pull request in Th30/brackets Feb 7, 2017

Fix mozilla#841: Highlight file upon right-click selection, fix 1

0c6eb49

Th30 referenced this pull request in Th30/brackets Feb 8, 2017

Fix mozilla#841: Highlight file upon right-click selection adjusted a…

4cbe355

…nd menu font fixed to match filetree, fix 2

Th30 referenced this pull request in Th30/brackets Feb 10, 2017

Fix mozilla#841: Highlight colors properly set, height fix

bfa965b

Th30 referenced this pull request in Th30/brackets Feb 10, 2017

Fix mozilla#841: Highlight colors properly set, height fix, minor cha…

f7cb2c8

…nges : Fix 2

Th30 referenced this pull request in Th30/brackets Feb 10, 2017

Fix mozilla#841: Indentation correction: Fix 3

49471fa

gideonthomas pushed a commit to gideonthomas/brackets that referenced this pull request Aug 31, 2017

Fix mozilla/thimble.mozilla.org#2252: Change position of Download act…

3f7d633

…ion in the right-click menu (adobe#841)

core-ai-bot mentioned this pull request Aug 29, 2021

[CLOSED] File Open Performance Tests brackets-archive/bracketsIssues#836

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

File Open Performance Tests #841

File Open Performance Tests #841

tvoliter commented May 8, 2012

jasonsanjose commented May 8, 2012

jasonsanjose May 9, 2012

tvoliter May 9, 2012

jasonsanjose May 10, 2012

tvoliter May 10, 2012

tvoliter May 10, 2012

jasonsanjose May 9, 2012

tvoliter May 10, 2012

jasonsanjose commented May 9, 2012

jasonsanjose commented May 10, 2012

jasonsanjose commented May 10, 2012

tvoliter commented May 10, 2012

jasonsanjose May 11, 2012

jasonsanjose May 11, 2012

tvoliter May 11, 2012


		describe("Performance Tests", function () {

		var testPath = SpecRunnerUtils.getTestPath("/../../../brackets-scenario/OpenFileTest/"),

File Open Performance Tests #841

File Open Performance Tests #841

Conversation

tvoliter commented May 8, 2012

jasonsanjose commented May 8, 2012

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jasonsanjose commented May 9, 2012

jasonsanjose commented May 10, 2012

jasonsanjose commented May 10, 2012

tvoliter commented May 10, 2012

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment