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

[huizhe wang]

Hi Christoph,

Aleksej has done great services to users of JDK 8 & 7 by backporting 
important changes from  JDK 9.  Please coordinate with Aleksej on this 
particular task.

Best,
Joe

On 3/10/2016 12:27 PM, Langer, Christoph wrote:
> Hi Joe,
>
> OK, thanks. I'll prepare the JDK 8 backport then.
>
> Best regards
> Christoph
>
>> -----Original Message-----
>> From: huizhe wang [mailto:huizhe.wang at oracle.com]
>> Sent: Donnerstag, 10. März 2016 19:07
>> 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 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