Hi Joe, OK, thanks. I'll prepare the JDK 8 backport then.
Best regards Christoph > -----Original Message----- > From: huizhe wang [mailto:huizhe.w...@oracle.com] > Sent: Donnerstag, 10. März 2016 19:07 > To: Langer, Christoph <christoph.lan...@sap.com> > Cc: core-libs-dev@openjdk.java.net > Subject: Re: [PING] RFR: JDK-8150704 XALAN: ERROR: 'No more DTM IDs are > available' when transforming with lots of temporary result trees > > Hi Christoph, > > Yes, an "Oops", sort of multitasking hazard, when I saw an all-test run > passed, I was a bit too eager to get it closed. We'll get those headers > fixed the next time we touch them. For the result files, I didn't quite > like them either, we could have picked up a few keywords to compare with > than the big files. But it's okay. > > And yes, it would be good to backport the patch to 8, or 7 for that matter. > > Best, > Joe > > On 3/10/2016 1:53 AM, Langer, Christoph wrote: > > Hi Joe, > > > > I've seen you have already submitted the change so I guess it's of no use > going through the Apache License headers now... > > > > I would also be willing to improve the tests although you should probably > > give > me some hint on how. Is it about improving the artificial xsl to stress the > fix? > Then some help on how to achieve that would really be appreciated. I also > don't like the way I read in reference result files and handle the line > endings. I > hope the test will run on Unix/Linuxes as well as on Windows. I tested in a > Windows Cygwin environment. > > > > Furthermore, any objections against backporting to 8? > > > > Thanks for reviewing and best regards > > Christoph > > > >> -----Original Message----- > >> From: huizhe wang [mailto:huizhe.w...@oracle.com] > >> Sent: Mittwoch, 9. März 2016 20:05 > >> To: Langer, Christoph <christoph.lan...@sap.com> > >> Cc: core-libs-dev@openjdk.java.net > >> Subject: Re: [PING] RFR: JDK-8150704 XALAN: ERROR: 'No more DTM IDs are > >> available' when transforming with lots of temporary result trees > >> > >> Hi Langer, > >> > >> Thanks for the update. I'll do an all-test run. I'm okay with the > >> TransformerTest, although it can be better. For the license header, I > >> meant to say the whole header that includes updating the Apache License > >> to its new format. You may update the webrev if you want. Otherwise, I > >> can do so when I check in once all tests pass. > >> > >> Thanks again, > >> Joe > >> > >> On 3/8/2016 3:59 PM, Langer, Christoph wrote: > >>> Hi Joe, > >>> > >>> answers inline. > >>> > >>>> Thanks for reporting and providing patch for the issue! Looks like a > >>>> nice solution that may potentially reduce memory requirement for some > >>>> large templates. Could you also verify that the patch also fixes > >>>> JDK-8150699 [1] that was created the same day as yours? > >>> Yes, which coincidence. The issue basically is the same. I've picked the > >>> bug > >> and marked it as duplicate > >>>> I assume the stylesheet is created to just illustrate the issue. If it's > >>>> a real use case, then it should have made the variable global to avoid > >>>> creating a lot of RTFs, and therefore avoid the whole "No more DTM IDs" > >>>> issue. It would make the process a lot more efficient. > >>> Yes, it is an artificial transformation which should recreate the issue. > >>> > >>>> Some classes, such as Sort.java, still contain the old header, please > >>>> update them with the new ones such as that in DOM.java. > >>> Did that. > >>> > >>>> The $Id section, such as the following, can all be removed, they were > >>>> from legacy repository, misleading since it implies the file was last > >>>> updated, in this case, in 2005: > >>>> > >>>> 20 /* > >>>> 21 * $Id: Sort.java,v 1.2.4.1 2005/09/12 11:08:12 pvedula Exp $ > >>>> 22 */ > >>> Did that as well. > >>> > >>>> For the new test, it's probably better to add some kind of assertion in > >>>> the test, e.g. expected result, than failing on a broad Exception. What > >>>> if the test passes but the transform operation isn't because of the > changes? > >>> I've modified that part, asserting that the result matches a reference. > >>> > >>>> The test is also not sufficient. The release methods seem to be okay. > >>>> However, they don't seem to have been fully exercised in the test (only > >>>> simple RTs were created?). In that sense, the sample attached in > >>>> JDK-8150699 provided an opportunity to better verify the changes. > >>> Yes, I had a hard time creating an artificial scenario which would > reproduce > >> the issue and would also stress all places. I was rather running into > >> StackOverflows than out of DTM IDs. Eventually I managed to create > something > >> but obviously not comprehensive enough. I also had some customer data > which > >> I was eventually allowed to publish as testcase but the data was quite > >> large > and > >> the xsl very complex so the transformation would run very long. I have now > >> included the sample from JDK-8150699 into the test as well. > >>>> It would be good to add some javadoc or dev notes to the test. While > >>>> consolidating tests (into TransformerTest), please make sure > >>>> notes/javadoc are copied over or added. > >>> I added a sentence of documentation for my testcase. For the consolidated > >> ones I now copied what was there and added a dummy summary text for the > >> tests where nothing was existing before. > >>> This is the new webrev: > >>> http://cr.openjdk.java.net/~clanger/webrevs/8150704.2/ > >>> > >>> I just ran the tests out of javax/xml/jaxp/unittest/transform. Maybe you > will > >> want to do some more testing before pushing, e.g. JCK. > >>> Let me know if I should do some further adaptions. > >>> > >>> Thanks > >>> Christoph
