Re: [OpenJDK 2D-Dev] Review Reqeust for Bug 100068 - SunGraphics2D exposes a reference to itself while non fully initialised
Sorry, after the flurry of emails here on other topics I just realized
this was getting buried.
The latest changes look good to me.
I just did a "one last grep through the sources" pass and noticed that
X11SD and GDIWindowSD also call getRenderLoops. They always call it,
though, because they install text pipes that will likely need it.
Perhaps we should check it for null before bothering with the call? I
looked quickly through and I can't totally vouch for the fact that the
loops are always needed unconditionally, but it did look likely. Also,
I think we've likely always had an extra call to validate the loops in
there so this is nothing new, but since we never were setting them to
null before we started we never had a way to check "have they been
updated by someone?" before.
I think we could add a quick "if (loops == null) {...}" in those 2
classes and maybe add a comment "// assert(some pipe will always be a
LoopBasedPipe)" to indicate that we aren't checking the types of the
pipes because we think it is always true in case that assumption changes
in the future. Does that make sense?
(Again, this isn't a new problem - just an opportunity to fix up some
old redundancy.)
...jim
Mario Torre wrote:
Il 17/09/2009 22:27, Jim Graham ha scritto:
http://cr.openjdk.java.net/~neugens/100068/webrev.08/
:)
Mario
Re: [OpenJDK 2D-Dev] Review Request for 6879044
There is no change to the log messages emitted from awt/2d/swing. The only impact to the end users is if they enable logging by modifying the system-wide logging.properties (see below); otherwise, the end users will not notice any change due to this fix. The following are the common ways to enable logging: 1) set -Djava.util.logging.config.file= system properties in the command-line This fix has no impact to the end user. Typically for debugging purpose, a user will have their custom logging.properties instead of modify the system-wide logging.properties as they would impact all Java apps. 2) Modify the system-wide logging.properties in the JRE local copy ( /lib/logging.properties) This fix has impact to the end user and they will need to add -Djava.util.logging.config.file=/lib/logging.properties option in the command-line. 3) Have their own java.util.logging.config.class implementation This fix has no impact to the end user. Thanks Mandy Igor Nekrestyanov wrote: We have discussed this with Anthony and Andrei offline and they do not seem to have any blocking concerns. There is one suggestion though - there should be simple explanation for "are there any changes required on the end user side to enable logging and if so, what are they". Logging is mostly used to get details from remote user and ideally instructions that were sent to such users (i.e. "if you have focus-related issue please do this and send us this") should not change (or it should be explained how they are changing). Anthony/Andrei, please speak up if you have other concerns. -igor On 9/23/09 5:08 AM, Anthony Petrov wrote: On 09/23/2009 03:41 PM, Alan Bateman wrote: If AWT initialized the loggers lazily, and only did it when the logging is actually enabled (checking for some system property, or whatever other way), would we still be statically linked to the j.u.logging package in case of a regular client application that does not use/enable logging explicitly? Yes, minimally they should be created lazily (this is one of the suggestions that Mandy listed in 6880089). That would at least avoid initializing all these loggers. The static dependency remains. Ah, I just looked over the PlatformLogger code, and now I see it uses reflection to access the j.u.logging.* classes. Now this point is clear. Have it been discussed whether that is feasible to modify the VM in order to eliminate the static dependency if a particular object never gets initialized? It just looks kind of artificial to create such proxy classes. I bet we'll need plenty of them for other modules. Won't the code be messed up too much? -- best regards, Anthony
Re: [OpenJDK 2D-Dev] Review Request for 6879044
We have discussed this with Anthony and Andrei offline and they do not seem to have any blocking concerns. There is one suggestion though - there should be simple explanation for "are there any changes required on the end user side to enable logging and if so, what are they". Logging is mostly used to get details from remote user and ideally instructions that were sent to such users (i.e. "if you have focus-related issue please do this and send us this") should not change (or it should be explained how they are changing). Anthony/Andrei, please speak up if you have other concerns. -igor On 9/23/09 5:08 AM, Anthony Petrov wrote: On 09/23/2009 03:41 PM, Alan Bateman wrote: If AWT initialized the loggers lazily, and only did it when the logging is actually enabled (checking for some system property, or whatever other way), would we still be statically linked to the j.u.logging package in case of a regular client application that does not use/enable logging explicitly? Yes, minimally they should be created lazily (this is one of the suggestions that Mandy listed in 6880089). That would at least avoid initializing all these loggers. The static dependency remains. Ah, I just looked over the PlatformLogger code, and now I see it uses reflection to access the j.u.logging.* classes. Now this point is clear. Have it been discussed whether that is feasible to modify the VM in order to eliminate the static dependency if a particular object never gets initialized? It just looks kind of artificial to create such proxy classes. I bet we'll need plenty of them for other modules. Won't the code be messed up too much? -- best regards, Anthony
Re: [OpenJDK 2D-Dev] Review Request for 6879044
We've chatted with Igor and I see no concern. Ok from my side. Thanks, Andrei Andrei V. Dmitriev wrote: Igor Nekrestyanov wrote: Let me clarify this as it is does not directly translate into 3x footprint. But savings are not just "size of classes to be loaded". Common "core" classes are also included into shared archive (classes.jsa) and removing logging classes from the "core" will reduce size of classes.jsa (which is mmap-ed to the memory on Windows). Note that we may read same class from rt.jar too if it happens to be in the same block as other class we need. Oh, I got it. In other words you are trying not to cache Loggers in classes.jsa and nothing else classes.jsa will be mmap-ed to the memory? That sounds promising. It will also reduce java quickstarter "read set" and this will make jqs more user friendly (less load on the system, smaller portion of disk cache taken for java stuff). None of these changes is "stunning" per se. But they sum up. (This does not mean that Mandy's approach is the only way to go, if someone can suggest how to improve logging package to get same savings and preserve backward compatibility - this will be even better.) We could make local to AWT changes with respect to loggers as described in 7). Thanks, Andrei -igor On 9/22/09 11:00 AM, Igor Nekrestyanov wrote: BTW, note that reducing set of required core classes will save 3x of memory at least (think of jqs and classes.jsa in addition to rt.jar (on windows)). -igor On 9/22/09 10:19 AM, Mandy Chung wrote: (I took the core-libs-dev off as this is about awt/2d/swing discussion). The main question is whether the client stack wants to require the dependency on logging when the JDK is broken down into fine-grained module. It is fine to wait until the jigsaw module system is ready to provide the performance numbers for you to evaluate. Some clarification inlined below. Andrei Dmitriev wrote: Hi Mandy, Oleg, I'm going to summarize pros and cons of this change just to make judge (basically for myself). 1) we can't expect a significant memory gain (the numbers are ~90Kb). That's a minus. I would not say it's a minus as it doesn't have negative impact. 2) we decouple awt/swing/2d with logging package which is a Plus in a view of jigsaw. See 6) for more. 3) we don't have time measures for this change. That's a minus. This statement isn't true. This should be "no significant perf improvement" for this fix and the perf improvment is not the goal for this fix. I did run the refworkload startup benchmark. But the numbers confirm that there is no significant improvement as expected. == mchung.baseline.b70: Benchmark SamplesMean Stdev Geomean Weight startup3 302.33 0.05 Framer 300.30 0.01 0.03 JEdit 301.71 0.11 0.30 LimeWire 302.21 0.06 0.30 NetBeans 307.38 0.14 0.30 Noop 300.11 0.00 0.03 XFramer300.30 0.00 0.04 == mchung.platform.logging: Benchmark SamplesMean Stdev %Diff P Significant startup3 302.34 0.05 -0.22 0.684 * Framer 300.30 0.000.33 0.326 * JEdit 301.70 0.090.99 0.522 * LimeWire 302.24 0.05 -1.56 0.025 * NetBeans 307.37 0.060.08 0.833 * Noop 300.11 0.02 -3.94 0.326 * XFramer300.30 0.000.22 0.310 * == * - Not Significant: A non-zero %Diff for the mean could be noise. If the %Diff is 0, an actual difference may still exist. In either case, more samples would be needed to detect an actual difference in sample means. 4) nobody measured anything else than Framer and SwingSet. That's a minus. As I said, the fix is to eliminate the dependency on logging. I'm not sure what measurement you want to do. 5) we lose flexibility on debugging. That's a minus. This statement isn't true. AWT debugging ability is unchanged. Mandy 6) this fix don't decouple anything else awt/swing/2d. I made a grep on "Logger.getLogger" and there are entries from xml, jmx, etc. That means we have to deal with them as well too (as it causes the class to be loaded anyway), if we aware of jigsaw. 7) In most cases AWT initiates classes statically but basically may want to do that laz
