Hopefully this is it.
http://gwt-code-reviews.appspot.com/1427807/diff/3007/dev/core/src/com/google/gwt/dev/util/log/speedtracer/SpeedTracerLogger.java
File
dev/core/src/com/google/gwt/dev/util/log/speedtracer/SpeedTracerLogger.java
(right):
http://gwt-code-reviews.appspot.com/1427807/diff/3007/dev/core/src/com/google/gwt/dev/util/log/speedtracer/SpeedTracerLogger.java#newcode672
dev/core/src/com/google/gwt/dev/util/log/speedtracer/SpeedTracerLogger.java:672:
gcMXBeans = null;
On 2011/05/05 16:45:54, tobyr wrote:
On 2011/05/05 16:06:21, Josh Humphries wrote:
> Since these members are declared "final", compiler complains if
there is no
> explicit assignment in constructor. That's why these null
assignments are
here.
>
> Should I remove the final modifier so we can clean up the
constructor?
>
> On 2011/05/04 22:22:05, tobyr wrote:
> > Looking at this a second time, there are lots of fields that are
seemingly
set
> > to null for no obvious reason. In fact, below there is an else
block just to
> set
> > fields to null. We should just drop all these null assignments as
they are
> just
> > adding code bloat.
>
Hrm, that's pretty crappy language behavior. While finals are nice, I
think I'd
rather see us get rid of many lines of code and extra conditional
blocks.
Done.
http://gwt-code-reviews.appspot.com/1427807/diff/10002/dev/core/src/com/google/gwt/dev/util/log/dashboard/DashboardNotifierFactory.java
File
dev/core/src/com/google/gwt/dev/util/log/dashboard/DashboardNotifierFactory.java
(right):
http://gwt-code-reviews.appspot.com/1427807/diff/10002/dev/core/src/com/google/gwt/dev/util/log/dashboard/DashboardNotifierFactory.java#newcode27
dev/core/src/com/google/gwt/dev/util/log/dashboard/DashboardNotifierFactory.java:27:
static {
On 2011/05/05 16:45:54, tobyr wrote:
Replace the entire body of this block (comments included) with just
setNotifier(createNotifier(System.getProperty("gwt.dashboard.notifierClass"))
Done.
http://gwt-code-reviews.appspot.com/1427807/diff/10002/dev/core/src/com/google/gwt/dev/util/log/dashboard/DashboardNotifierFactory.java#newcode67
dev/core/src/com/google/gwt/dev/util/log/dashboard/DashboardNotifierFactory.java:67:
static DashboardNotifier setNotifierClass(String className) {
On 2011/05/05 16:45:54, tobyr wrote:
Almost. I'm looking for this to be "createNotifier".
Done.
http://gwt-code-reviews.appspot.com/1427807/diff/10002/dev/core/src/com/google/gwt/dev/util/log/dashboard/DashboardNotifierFactory.java#newcode82
dev/core/src/com/google/gwt/dev/util/log/dashboard/DashboardNotifierFactory.java:82:
setNotifier(notifier);
On 2011/05/05 16:45:54, tobyr wrote:
Drop this call to setNotifier.
Done.
http://gwt-code-reviews.appspot.com/1427807/diff/10002/dev/core/src/com/google/gwt/dev/util/log/speedtracer/SpeedTracerLogger.java
File
dev/core/src/com/google/gwt/dev/util/log/speedtracer/SpeedTracerLogger.java
(right):
http://gwt-code-reviews.appspot.com/1427807/diff/10002/dev/core/src/com/google/gwt/dev/util/log/speedtracer/SpeedTracerLogger.java#newcode66
dev/core/src/com/google/gwt/dev/util/log/speedtracer/SpeedTracerLogger.java:66:
private static final boolean logProcessCpuTime =
On 2011/05/05 16:45:54, tobyr wrote:
Can we add a test to the constructor that ensures that only one of
logProcessCpuTime and logThreadCpuTime is set?
Done.
http://gwt-code-reviews.appspot.com/1427807/diff/10002/dev/core/src/com/google/gwt/dev/util/log/speedtracer/SpeedTracerLogger.java#newcode331
dev/core/src/com/google/gwt/dev/util/log/speedtracer/SpeedTracerLogger.java:331:
* @param refEvent the event during which the garbage collections
took place
Okay. Eclipse doesn't auto-format Javadoc very pretty. Info on Java
style guide for this (here
[http://www.corp.google.com/eng/doc/javaguide.html#javadoc]) doesn't
give specifics. I'm just used to using this format for readability.
I've changed it now. Let me know if this looks better (I tried to mimic
some other Javadoc in the same file that has multiple @param tags and
multi-line @param text).
On 2011/05/05 16:45:54, tobyr wrote:
Is your Eclipse formatting aligning these this way? I don't think we
do this
normally.
http://gwt-code-reviews.appspot.com/1427807/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
