[PING] RFR: JDK-8150704 XALAN: ERROR: 'No more DTM IDs are available' when transforming with lots of temporary result trees
Hi, Ping - any comments or reviews for this bugfix? Thanks Christoph From: Langer, Christoph Sent: Freitag, 26. Februar 2016 16:02 To: core-libs-dev@openjdk.java.net Subject: RFR: JDK-8150704 XALAN: ERROR: 'No more DTM IDs are available' when transforming with lots of temporary result trees Hi, I've created a fix proposal for the issue I have reported in this bug: https://bugs.openjdk.java.net/browse/JDK-8150704 The webrev can be found here: http://cr.openjdk.java.net/~clanger/webrevs/8150704.1/ The Xalan parser would eventually run out of DTM IDs if xsl transformations involve lots of temporary result trees. Those are never released although they could. A testcase is included for this. I've also done some cleanups in the Xalan code and in the tests. Thanks in advance for looking at this :) Best regards Christoph
Hi Christoph, 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? 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. Some classes, such as Sort.java, still contain the old header, please update them with the new ones such as that in DOM.java. 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 */ 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? 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. 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. [1] https://bugs.openjdk.java.net/browse/JDK-8150699 Thanks, Joe On 3/3/2016 11:50 PM, Langer, Christoph wrote:
Hi,
Ping - any comments or reviews for this bugfix?
Thanks Christoph
From: Langer, Christoph Sent: Freitag, 26. Februar 2016 16:02 To: core-libs-dev@openjdk.java.net Subject: RFR: JDK-8150704 XALAN: ERROR: 'No more DTM IDs are available' when transforming with lots of temporary result trees
Hi,
I've created a fix proposal for the issue I have reported in this bug: https://bugs.openjdk.java.net/browse/JDK-8150704
The webrev can be found here: http://cr.openjdk.java.net/~clanger/webrevs/8150704.1/
The Xalan parser would eventually run out of DTM IDs if xsl transformations involve lots of temporary result trees. Those are never released although they could. A testcase is included for this. I've also done some cleanups in the Xalan code and in the tests.
Thanks in advance for looking at this :)
Best regards Christoph
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
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
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@oracle.com] Sent: Mittwoch, 9. März 2016 20:05 To: Langer, Christoph <christoph.langer@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
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@oracle.com] Sent: Mittwoch, 9. März 2016 20:05 To: Langer, Christoph <christoph.langer@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
Hi Joe, OK, thanks. I'll prepare the JDK 8 backport then. Best regards Christoph
-----Original Message----- From: huizhe wang [mailto:huizhe.wang@oracle.com] Sent: Donnerstag, 10. März 2016 19:07 To: Langer, Christoph <christoph.langer@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.wang@oracle.com] Sent: Mittwoch, 9. März 2016 20:05 To: Langer, Christoph <christoph.langer@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
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@oracle.com] Sent: Donnerstag, 10. März 2016 19:07 To: Langer, Christoph <christoph.langer@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.wang@oracle.com] Sent: Mittwoch, 9. März 2016 20:05 To: Langer, Christoph <christoph.langer@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
participants (2)
-
huizhe wang
-
Langer, Christoph