[L] Change in dart/sdk[main]: Fine. Add 'produceErrors' statistics to 'analysis_statistics'.
Konstantin Shcheglov (Gerrit)
Konstantin Shcheglov added 1 comment![]( "Open in Gerrit")
Event.analysisStatistics(I will need to update `unified_analytics` package to use these new items. Maybe add generic `Map<String, Object?> eventData` instead, to be able to change it freely?
Related details
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Johnni Winther (Gerrit)
Johnni Winther voted and added 1 comment![]( "Open in Gerrit")
Votes added by Johnni Winther
| Code-Review | +1 |
1 comment
"tools_rev": "6866f9b19553625cc8af099d67aecbfb02067dcD",Is this intended?
Related details
- Brian Wilkerson
- Konstantin Shcheglov
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Brian Wilkerson (Gerrit)
Brian Wilkerson added 1 comment![]( "Open in Gerrit")
Event.analysisStatistics(I will need to update `unified_analytics` package to use these new items. Maybe add generic `Map<String, Object?> eventData` instead, to be able to change it freely?
Discussed in person.
But I have a question about the values. The event currently records timings for multiple analyses, but it looks like the rest of the data is for a specific analysis (with the exception of `withFineDependencies`, which can't change from one analysis to another). Which timing are the other values associated with?
Related details
- Konstantin Shcheglov
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Konstantin Shcheglov (Gerrit)
Konstantin Shcheglov added 1 comment![]( "Open in Gerrit")
PTAL, I added more information: about immediate / transitive file counts, and counts of changes to files. This should help us understand sizes of workspaces and how many / often files change.
Related details
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Brian Wilkerson (Gerrit)
Brian Wilkerson added 3 comments![]( "Open in Gerrit")
final Set<String> uniqueChangedFiles = {};I don't understand what information this is giving us. As I understand it, this is used to compute the number of unique files across an arbitrary number of independent analysis working periods.
I could understand keeping the number of changed files in each working period (as a percentile graph) because it might be useful to know how many files were changed when each analysis is performed.
But doesn't the number become meaningless if we're merging data from multiple periods?
(Same comment applies to `uniqueRemovedFiles` and the potential / actual file and line counts.)
Also, is the number of added files of interest?
int produceErrorsMs = 0;I don't understand this either. If we look at the data and can conclude that across 1 million users it took 5 weeks of compute time to produce errors, what does that tell us?
On the other hand, if we know that the mean time to compute errors is 1ms per 10K lines of code, then I think that gives us something interesting. But we can't compute that unless we store the timing information, normalized by lines of code, as a percentile.
int produceErrorsPotentialFileCount = 0;I'm wondering why the 'produceErrors' prefix is useful here. Is there a different potential file count for different phases of analysis?
Related details
- Konstantin Shcheglov
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Konstantin Shcheglov (Gerrit)
Konstantin Shcheglov added 3 comments![]( "Open in Gerrit")
final Set<String> uniqueChangedFiles = {};I don't understand what information this is giving us. As I understand it, this is used to compute the number of unique files across an arbitrary number of independent analysis working periods.
I could understand keeping the number of changed files in each working period (as a percentile graph) because it might be useful to know how many files were changed when each analysis is performed.
But doesn't the number become meaningless if we're merging data from multiple periods?
(Same comment applies to `uniqueRemovedFiles` and the potential / actual file and line counts.)
Also, is the number of added files of interest?
The number of added files is somewhat interesting, but it might be a few thousand files spike at the start, and I thought that maybe we don't want to pay for it.
The total number of changed files is interesting because it will tell us how diverse was area of work during this 30 minutes period: was it 1-2 files constantly edited, or many files maybe auto-generated.
The percentiles are not interesting, at least for me: it is almost always 1, while editing.
Removed files are for different purpose: I need to know how often it happens, and whether it is worth handling removing better, currently it is somewhat expensive.
int produceErrorsMs = 0;I don't understand this either. If we look at the data and can conclude that across 1 million users it took 5 weeks of compute time to produce errors, what does that tell us?
On the other hand, if we know that the mean time to compute errors is 1ms per 10K lines of code, then I think that gives us something interesting. But we can't compute that unless we store the timing information, normalized by lines of code, as a percentile.
We can compute `produceErrorsMs / produceErrorsPotentialFileLineCount`, this will give us time per line.
Measuring it later in weeks also sounds fun :-)
int produceErrorsPotentialFileCount = 0;I'm wondering why the 'produceErrors' prefix is useful here. Is there a different potential file count for different phases of analysis?
I think it is useful to keep it organized with prefix to the names, for clarity. I can imagine we could measure timings and counts for element requests in the future.
Related details
- Brian Wilkerson
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Brian Wilkerson (Gerrit)
Brian Wilkerson voted and added 4 comments![]( "Open in Gerrit")
Votes added by Brian Wilkerson
| Code-Review | +1 |
4 comments
Event.analysisStatistics(Brian WilkersonI will need to update `unified_analytics` package to use these new items. Maybe add generic `Map<String, Object?> eventData` instead, to be able to change it freely?
Discussed in person.
But I have a question about the values. The event currently records timings for multiple analyses, but it looks like the rest of the data is for a specific analysis (with the exception of `withFineDependencies`, which can't change from one analysis to another). Which timing are the other values associated with?
Acknowledged
Konstantin ShcheglovI don't understand what information this is giving us. As I understand it, this is used to compute the number of unique files across an arbitrary number of independent analysis working periods.
I could understand keeping the number of changed files in each working period (as a percentile graph) because it might be useful to know how many files were changed when each analysis is performed.
But doesn't the number become meaningless if we're merging data from multiple periods?
(Same comment applies to `uniqueRemovedFiles` and the potential / actual file and line counts.)
Also, is the number of added files of interest?
The number of added files is somewhat interesting, but it might be a few thousand files spike at the start, and I thought that maybe we don't want to pay for it.
The total number of changed files is interesting because it will tell us how diverse was area of work during this 30 minutes period: was it 1-2 files constantly edited, or many files maybe auto-generated.
The percentiles are not interesting, at least for me: it is almost always 1, while editing.
Removed files are for different purpose: I need to know how often it happens, and whether it is worth handling removing better, currently it is somewhat expensive.
Acknowledged
Konstantin ShcheglovI don't understand this either. If we look at the data and can conclude that across 1 million users it took 5 weeks of compute time to produce errors, what does that tell us?
On the other hand, if we know that the mean time to compute errors is 1ms per 10K lines of code, then I think that gives us something interesting. But we can't compute that unless we store the timing information, normalized by lines of code, as a percentile.
We can compute `produceErrorsMs / produceErrorsPotentialFileLineCount`, this will give us time per line.
Measuring it later in weeks also sounds fun :-)
Acknowledged
Konstantin ShcheglovI'm wondering why the 'produceErrors' prefix is useful here. Is there a different potential file count for different phases of analysis?
I think it is useful to keep it organized with prefix to the names, for clarity. I can imagine we could measure timings and counts for element requests in the future.
- Konstantin Shcheglov
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Konstantin Shcheglov (Gerrit)
Konstantin Shcheglov added 1 comment![]( "Open in Gerrit")
"tools_rev": "6866f9b19553625cc8af099d67aecbfb02067dcD",Konstantin ShcheglovIs this intended?
Yes, I wanted to mark `DEPS` as changed, but the specific hash was not yet known, because my PR for `unified_analytics` was not merged yet. Updated to real now.
Related details
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Konstantin Shcheglov (Gerrit)
Konstantin Shcheglov voted Commit-Queue+2![]( "Open in Gerrit")
| Commit-Queue | +2 |
Related details
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Commit Queue (Gerrit)
Commit Queue submitted the change with unreviewed changes![]( "Open in Gerrit")
Unreviewed changes
7 is the latest approved patch-set.
The change was submitted with unreviewed changes in the following files:
```
The name of the file: pkg/analysis_server/lib/src/analytics/analytics_manager.dart
Insertions: 1, Deletions: 1.
The diff is too large to show. Please review the diff.
```
```
The name of the file: DEPS
Insertions: 2, Deletions: 2.
The diff is too large to show. Please review the diff.
```
Change information
Fine. Add 'produceErrors' statistics to 'analysis_statistics'.
- M DEPS
- M pkg/analysis_server/lib/src/analysis_server.dart
- M pkg/analysis_server/lib/src/analytics/analytics_manager.dart
- M pkg/analysis_server/lib/src/analytics/session_data.dart
- M pkg/analysis_server/test/src/analytics/analytics_manager_test.dart
- M pkg/analyzer/lib/src/dart/analysis/driver.dart
- M pkg/analyzer/lib/src/dart/analysis/status.dart
Code-Review: +1 by Brian Wilkerson, +1 by Johnni Winther
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |