[PING] RFR: JDK-8150704 XALAN: ERROR: 'No more DTM IDs are available' when transforming with lots of temporary result trees
huizhe.wang at oracle.com
Wed Mar 9 19:05:24 UTC 2016
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.
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  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 188.8.131.52 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:
> 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.
More information about the core-libs-dev