Separated out different classes to worked on improving readability #2
Conversation
patrick-sko
changed the base branch from
implementation_of_reporter_without_commandLineArgs
to
master
src/com/google/ProfilingReport.java
Outdated
| * The instrumentation will send the report in a JSON format where the JSON is a dictionary of the | ||
| * encoded params to an object of the data collected. Initially this will just be the frequency. | ||
| */ | ||
| public static class ProfilingData { |
There was a problem hiding this comment.
Can we also make this AutoValue?
It might make the class definition slightly larger for now, but will be good if we need to update the class in the future.
There was a problem hiding this comment.
I'm not sure if I am not using AutoValue incorrectly, but marking this class as AutoValue will break JSON deserialization. This class is used to convert the executionResults.json file for reference.
There was a problem hiding this comment.
Interesting, that's fine then.
Could you just add a short comment explaining why this can't be AutoValue?
src/com/google/ProfilingReport.java
Outdated
|
|
||
| abstract float percentOfBranchesExecuted(); | ||
|
|
||
| abstract Map<String, List<ProfilingResult>> profilingDataPerFunction(); |
There was a problem hiding this comment.
Consider using a MultiMap here
https://guava.dev/releases/23.0/api/docs/com/google/common/collect/Multimap.html
It's a cleaner abstraction than having the value type be a List
There was a problem hiding this comment.
Guava really does have a interface for everything.
There was a problem hiding this comment.
As much as a like this interface, and have tried to make it work, it seems very difficult to use with JSON serialization. This field needs to be a Map of String keys to a list of ProfilingResult values, otherwise the Gson will not serialize properly. Any work around I believe will require changes to how Gson serializes data which I don't believe is worth it.
|
Hello, is there any plan this can be merged into the Closure Compiler repo and becomes part of the compiler itself? |
This PR also contains three different files (Instrumentation Mapping, Execution results, and the final JSON) which are examples of what the format looks like. This way you can look in greater detail at what the reporter parsed and generates in case we might want to change something.
I also added command line arguments to the reporter which is in this PR.