analysis server performance measurement - work in progress

analysis server performance measurement - work in progress
still need to debug some failing requests,
verify responses match expected responses,
track notifications to determine what was analyzed,
add docs, and ...

BUG=
R=brianwilkerson@google.com

Committed: https://github.com/dart-lang/sdk/commit/108f6c114b880aeed4591d1e8ca909f279a71dc8

[danrubel, 2015-06-15 16:44:52]

danrubel@google.com changed reviewers:
+ brianwilkerson@google.com

[Brian Wilkerson, 2015-06-15 17:37:47]

LGTM

https://codereview.chromium.org/1182933005/diff/1/pkg/analysis_server/test/pe...
File pkg/analysis_server/test/performance/input_converter.dart (right):

https://codereview.chromium.org/1182933005/diff/1/pkg/analysis_server/test/pe...
pkg/analysis_server/test/performance/input_converter.dart:88: if (method ==
'analysis.getHover' ||
This is going to get us in trouble. We should minimally add a TODO comment to
clean this up so that we don't need a list that is easy to get out of sync with
the actual implementation (and that can deal with contributed handlers).

https://codereview.chromium.org/1182933005/diff/1/pkg/analysis_server/test/pe...
pkg/analysis_server/test/performance/input_converter.dart:113: throw '$e\n  in
$json';
Personally, I dislike throwing anything other than an exception. Even though
it's something of an abuse, you might consider throwing an AnalysisException
because it knows how to display the wrapped exception's stack trace, which will
probably make debugging easier.

  throw new AnalysisException("error in $json", new CaughtException(e, s));

https://codereview.chromium.org/1182933005/diff/1/pkg/analysis_server/test/pe...
pkg/analysis_server/test/performance/input_converter.dart:205: Map<String,
dynamic> translateSrcPaths(Map<String, dynamic> origJson) {
Consider "translateSrcPaths" --> "translateSrcPathsInMap" for consistency.

https://codereview.chromium.org/1182933005/diff/1/pkg/analysis_server/test/pe...
pkg/analysis_server/test/performance/input_converter.dart:230: if (value is
String) {
This logic is repeated at least 4 times. It should be moved to a utility method.

https://codereview.chromium.org/1182933005/diff/1/pkg/analysis_server/test/pe...
File pkg/analysis_server/test/performance/instrumentation_input_converter.dart
(right):

https://codereview.chromium.org/1182933005/diff/1/pkg/analysis_server/test/pe...
pkg/analysis_server/test/performance/instrumentation_input_converter.dart:53: if
(opCode == 'Noti') {
Use constants in InstrumentationService to protect against inconsistencies.

[danrubel, 2015-06-17 15:35:32]

https://codereview.chromium.org/1182933005/diff/1/pkg/analysis_server/test/pe...
File pkg/analysis_server/test/performance/input_converter.dart (right):

https://codereview.chromium.org/1182933005/diff/1/pkg/analysis_server/test/pe...
pkg/analysis_server/test/performance/input_converter.dart:88: if (method ==
'analysis.getHover' ||
On 2015/06/15 17:37:47, Brian Wilkerson wrote:
> This is going to get us in trouble. We should minimally add a TODO comment to
> clean this up so that we don't need a list that is easy to get out of sync
with
> the actual implementation (and that can deal with contributed handlers).

Agreed. I implemented it this way for now so that I could review each new api
being called and think about its implications. In the future, I can just apply a
transform to every request and send it to the server.

https://codereview.chromium.org/1182933005/diff/1/pkg/analysis_server/test/pe...
pkg/analysis_server/test/performance/input_converter.dart:113: throw '$e\n  in
$json';
On 2015/06/15 17:37:47, Brian Wilkerson wrote:
> Personally, I dislike throwing anything other than an exception. Even though
> it's something of an abuse, you might consider throwing an AnalysisException
> because it knows how to display the wrapped exception's stack trace, which
will
> probably make debugging easier.
> 
>   throw new AnalysisException("error in $json", new CaughtException(e, s));

Good suggestion. Done.

https://codereview.chromium.org/1182933005/diff/1/pkg/analysis_server/test/pe...
pkg/analysis_server/test/performance/input_converter.dart:205: Map<String,
dynamic> translateSrcPaths(Map<String, dynamic> origJson) {
On 2015/06/15 17:37:47, Brian Wilkerson wrote:
> Consider "translateSrcPaths" --> "translateSrcPathsInMap" for consistency.

Done.

https://codereview.chromium.org/1182933005/diff/1/pkg/analysis_server/test/pe...
pkg/analysis_server/test/performance/input_converter.dart:230: if (value is
String) {
On 2015/06/15 17:37:47, Brian Wilkerson wrote:
> This logic is repeated at least 4 times. It should be moved to a utility
method.

Good point. Reworked this group of methods to remove the duplicate code.

https://codereview.chromium.org/1182933005/diff/1/pkg/analysis_server/test/pe...
File pkg/analysis_server/test/performance/instrumentation_input_converter.dart
(right):

https://codereview.chromium.org/1182933005/diff/1/pkg/analysis_server/test/pe...
pkg/analysis_server/test/performance/instrumentation_input_converter.dart:53: if
(opCode == 'Noti') {
On 2015/06/15 17:37:47, Brian Wilkerson wrote:
> Use constants in InstrumentationService to protect against inconsistencies.

Good point. Done.

[danrubel, 2015-06-17 15:35:52]

Committed patchset #2 (id:20001) manually as
108f6c114b880aeed4591d1e8ca909f279a71dc8 (presubmit successful).