[PING] RFR: JDK-8150704 XALAN: ERROR: 'No more DTM IDs are available' when transforming with lots of temporary result trees

[huizhe wang]

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.wang at oracle.com]
>> Sent: Mittwoch, 9. März 2016 20:05
>> To: Langer, Christoph <christoph.langer at sap.com>
>> Cc: core-libs-dev at 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