Daniel, We're happy to contribute. Like you, we had a customer complaint, which is why this happened.
My suspicion is that we don't have access to the VM/NSK test suite. Feel free to run the patch against it. Jeremy On Mon, Jun 14, 2010 at 10:45 AM, Daniel D. Daugherty <[email protected]> wrote: > On 6/14/2010 11:30 AM, Jeremy Manson wrote: >> >> Daniel, >> >> The fix hasn't made it to OpenJDK6. We were planning on pushing it to >> OpenJDK6/7, but we haven't had time for it yet. If your fix is better >> (I haven't had a chance to look at it), then we'll happily drop ours >> in favor of yours. >> > > I will be looking at your fix today. Mine is a brute force big hammer > style fix that I'm not fond of, but does the job. Your fix will likely > address Eamonn's review comments and also solve the potential performance > impact that mine would have in systems with lots of Loggers. > >> For testing: I hand tested it with the "create lots of anonymous >> loggers" test. My major observation was that creating that many weak >> references and rebuilding the internal data structures from scratch >> repeatedly can bring a system to its knees. This is why the >> weakReferencesProcessed variable exists - we don't rebuild the >> internal data structures with every additional logger that we lose. >> > > I'll be sure to keep my eyes open for that part of the fix. > > >> We also ran jtreg, which didn't seem to have any problems. >> > > There are only 10 or so tests in JTREG. The VM/NSK logging test suite > has 550+ tests and it caught my breakage due to the currently legal > Logger loop in one of the tests. > > >> The doc fixes: Our secret goal was to sneak those in without having to >> file a separate bug, but I guess you caught us. ;) >> > > Perhaps I can sweep those up for you. We'll have to see how it goes. > > Thanks again for contributing the fix! > > Dan > > >> Jeremy >> >> On Mon, Jun 14, 2010 at 9:46 AM, Daniel D. Daugherty >> <[email protected]> wrote: >> >>> >>> On 6/11/2010 4:41 PM, Martin Buchholz wrote: >>> >>>> >>>> On Fri, Jun 11, 2010 at 14:46, Daniel D. Daugherty >>>> <[email protected]> wrote: >>>> >>>> >>>>> >>>>> Jeremy, >>>>> >>>>> I'm definitely interested in learning about your approach to this >>>>> issue. >>>>> >>>>> >>>> >>>> Here's the patch against openjdk6 by Jeremy. >>>> http://cr.openjdk.java.net/~martin/WeakLogger-jeremymanson.patch >>>> (It would take a bit of merging to port to openjdk7) >>>> >>>> Feel free to take anything from our change. >>>> Apologies for the perforce-isms. >>>> >>>> Martin >>>> >>>> >>> >>> Jeremy and Martin, >>> >>> Thanks for the proposed fix. A couple of questions: >>> >>> - This changeset is private to Google right now, correct? As in >>> it hasn't made it into OpenJDK6 yet. >>> - Do you plan on pushing this changeset to OpenJDK6? >>> - What kind of testing has been done on it? >>> >>> Thanks for the offer for the code. I'll start wading through the >>> diffs today. Because this is an escalated issue, I will likely >>> be taking just the code and comments directly related to the >>> problem at hand. The JavaDoc fixes, even though they are useful, >>> will have to wait for a different changeset. >>> >>> Thanks for jumping in on this thread. >>> >>> Dan >>> >>> >>> >
