Closed ithena closed 11 years ago
keshavarzi
commented
11 years ago OK. But the comparison has become costly meanwhile. Could you please increase the heap memory to maximum and run the tests again? I'm just interested if there is a bug or is it only because of the high memory usage.
ithena
commented
11 years ago My VM has a heap of 3GB, and it does not fill up completely. Although the comparison algorithm does use much more memory (now +-1GB, do not know exactly before but it was not more than a few 100 MBs) than before.
keshavarzi
commented
11 years ago There is probably a loop in the new SequenceDiffGenerator, which causes the error if a schema Element and a ComplexType have cycling references to each other. A fix could be restoring the compared object in a list in the context and avoiding the comparison further. A fix will be added soon.
ithena
commented
11 years ago Ok, thx for the quick response
keshavarzi
commented
11 years ago It should work fine with 515037faa21765fe0b6a48c12edf9777988540b2
ithena
commented
10 years ago @keshavarzi , I am still witnessing the StackOverflowException. I will try to look deeper into what exactly is causing the issue.
keshavarzi
commented
10 years ago Could you provide us with some sample files to reconstruct the issue?
ithena
commented
10 years ago I am having a look into the problem, and will add extra information as I find it.
We are also witnessing a noticeable slow down of the compare algorithm. Where the complete compare of 300 WSDLs would take 2 minutes (8 threads), some of the single comparisons now take 50 to 150 seconds.
ithena
commented
10 years ago The issue occurs due to the extra ref based comparison in ElementDiffGenerator.compare4WSDL().
What is the difference when simply comparing the Schema's and comparing the Schema's when we start from a WSDL file?
keshavarzi
commented
10 years ago Consider:
<element name="foo" type="tns:bar">
<complexType name="bar">...</complexType>and
<element name="foo" type="tns:bar">
<complexType name="bar">...with some changes...</complexType>If you simply comare schemas, there are differences only in the complexType. The elements are the same. But from a WSDL it is the element which is interesting. So if two elements seems to be equal, also the content of them will be compared. In this case you would have: Element foo has changed because type bar has changed...
ithena
commented
10 years ago Our Schema's contain a recursive definition, which causes the StackOverflowError.
<xsd:element name="A" type="typeA"/>
<xsd:complexType name="typeA">
<xsd:sequence>
<xsd:element ref="B"/>
</xsd:sequence>
</xsd:complexType>
<xsd:element name="B" type="typeB"/>
<xsd:complexType name="typeB">
<xsd:sequence>
<xsd:element name="listOfB" type="listOfAType"/>
</xsd:sequence>
</xsd:complexType>
<xsd:complexType name="listOfAType">
<xsd:sequence>
<xsd:element ref="A"/>
</xsd:sequence>
</xsd:complexType>
OK. I will have a look...
ithena
commented
10 years ago Thanks, I will also have a go at it using the DiffGeneratorContext
keshavarzi
commented
10 years ago We actually have a check to avoid cycling dependencies. Perhaps there is a bug in there!
ithena
commented
10 years ago The compare4WSDL method contains the following comment. So I assume the StackOverflowError is currently expected behavior.
//Recursion is possible if an element references a type, which returns to the element in further steps.
List<Difference> compare4WSDL() {Could this be caused by AbstractModelDiffGenerator.compareUnit() not reusing the DiffGeneratorContext when creating a new ElementsDiffGenerator?
After this call DiffGeneratorContext.visited is empty, while before there were already elements added.
keshavarzi
commented
10 years ago @ithena
I think you've got it! Please replave the line 32 with
def diffs = new ElementsDiffGenerator(a: a.elements, b: b.elements, generator: generator, ctx: ctx.clone()).compare()
and test if it works. This should be the bug.
ithena
commented
10 years ago This moves the problem one step further. There are probably some more places where the DiffGeneratorContext needs to be set.
ithena
commented
10 years ago In ComplexTypeDiffGenerator.compareModel() the DiffGeneratorContext contains multiple visited elements, while the DiffGeneratorContext of the generator used here has no visited elements.
keshavarzi
commented
10 years ago This is not necessarily false! The context should forget the visited as you leave a specific path of the XML tree. So the same type or element could be compared from another position. Do you have also cycling attribute definitions in your schema documents?
ithena
commented
10 years ago I added a test that shows the StackoverflowError to a separate branch.
keshavarzi
commented
10 years ago OK, I'll see.
keshavarzi
commented
10 years ago I've got it. Some times the easiest bugs are the hardest to find! If you agree, I would commit your test data to the project. It was shame to lose it.
ithena
commented
10 years ago Glad if I can help with the test data.
Thx for the quick response and fix!
keshavarzi
commented
10 years ago Fixed in cbe559dd5ec98bce9d9fc9e760e3ea5a7e46541e
When comparing WSDLs with a large type tree that contains Elements refs a StackOverFlowException occurs. When disabling the new code in ElementDiffGenerator the exception does not occur.
Stacktrace: