I think I have found a bug with Fast Serialization when it is used with cyclical entity references. It may be something other than cyclical references that is causing the problem, but...well...I have attached a sample app which shows the problem.

Which runtime lib build nr. are you using and which llblgen pro version ? If it's not the latest build, could you please check with the latest build?

edit: I found your other thread: http://www.llblgen.com/tinyforum/GotoMessage.aspx?MessageID=89729&ThreadID=7028

which suggests you're using the latest build.

Otis wrote:

Which runtime lib build nr. are you using and which llblgen pro version ? If it's not the latest build, could you please check with the latest build?

edit: I found your other thread: http://www.llblgen.com/tinyforum/GotoMessage.aspx?MessageID=89729&ThreadID=7028

which suggests you're using the latest build.

Yeah, I wasn't using the latest llblgen pro version or build in that sample app, I was in my real project (where I first encountered the bug), but not in that sample app. I have attached the sample app built with both the latest runtime and latest llblgen pro version used.

Will look into it. This might take a few days, as the code is from an external source (licensed) and we therefore likely need some extra time to get to the core of this issue as the code is very complex.

Otis wrote:

Will look into it. This might take a few days, as the code is from an external source (licensed) and we therefore likely need some extra time to get to the core of this issue as the code is very complex.

Yep.

I took one look at the FastSerializer code and ran away as fast as I could, no offense to Simon

Your repro case doesn't contain any data, so I've to rewrite your app in a console app in C# to be able to check the ObjectID's in the entity instances, to see if this works. The VB.NET debugger is not advanced enough to let me peek into elements in the graph at runtime...

Using C#, aCopyFS.ExecutionDependencies[0].DependsOnReportTable.ExecutionDependencies[ 0].DependsOnReportTable

is null which is even worse than you suggested. (latest runtime).

(I;ve renamed everything as:

var a = new ReportTableEntity() { Name = "a", Id=1 };
var b = new ReportTableEntity() { Name = "b", Id=2 };
var c = new ReportTableEntity() { Name = "c", Id=3 };

var ab = new ReportTableExecutionDependencyEntity() {Table = a, DependsOnReportTable = b};
var bc = new ReportTableExecutionDependencyEntity() {Table = b, DependsOnReportTable = c};
var ca = new ReportTableExecutionDependencyEntity() {Table = c, DependsOnReportTable = a};

It's cumbersome to try to find it, also because you didn't include the .lgp file so I can't regenerate the code in a different language, or using the latest templates. Anyway, I'll see if I can find why the elements aren't restored properly. I also can't see if you have hidden some relations, which is crucial here I think, if a relation is hidden on one side, it won't give proper results.

I do want to know from you if you have tried this with real data, as the objects are now all empty.

(edit) the deserialization of bc goes wrong, the reference to 'c' is null. (edit) for some silly reason, all entities are in the list of entities to serialize except bc... (edit) I think the optimization somewhere is a bit too optimistic. It ignores 'bc' as it thinks it's not referenced by anything else. I'll contact simon about this.

The problem is that it traverses the graph, and it does it in the order: A, AB, CA, B, C, BC. The thing is: when 'bc' is seen as a related entity of 'b' it is first placed in a queue of unreferenced entities, the idea I think is that if it's referenced by an entity it will be included anyway. The problem is that those referencing entities have already been processed (b and c) so it's ignored. I've to consult with Simon if it can be added regardless, or if it has to be threated differently.

In the meantime, use normal serialization for this particular graph.

Strange that I am the first one to notice this...that fastserialization code has been in use for quite a while, it seems like.

Normal serialization is fine right now though, I can wait.

mikeg22 wrote:

Strange that I am the first one to notice this...that fastserialization code has been in use for quite a while, it seems like.

Normal serialization is fine right now though, I can wait.

The problem is a specific edge case graph. When I analyzed the graph you presented in the debugger when stepping through the fast serializer code I recognized it from my early attempt to be clever in a depth first search based algo over the entity graph for saving: the algo uses two states for nodes: not visited and visited. But you need 3: not visited, visiting and visited. (the proper term for this is 'graph coloring')

The graph you presented contains multiple paths to the same element, C AND one path is longer than the other. This means that you could get the situation where 'C' is processed, but 'bc' isn't. and 'bc' isn't examined more than once, as it has to be accepted when 'c' is examined, but that doesn't happen in this case.

At least that's the conclusion I've drawn during the debugging so far. I hope to have a more clear answer soon.

Anyway, I've mailed Simon so I hope he responds soon, otherwise I've to make the changes and test whether they hold up in the unittests we have, though I don't like to rely on unit tests alone as they only reflect a small subset of the unlimited number of possible situations.

Otis wrote:

mikeg22 wrote:

Strange that I am the first one to notice this...that fastserialization code has been in use for quite a while, it seems like.

Normal serialization is fine right now though, I can wait.

The problem is a specific edge case graph. When I analyzed the graph you presented in the debugger when stepping through the fast serializer code I recognized it from my early attempt to be clever in a depth first search based algo over the entity graph for saving: the algo uses two states for nodes: not visited and visited. But you need 3: not visited, visiting and visited. (the proper term for this is 'graph coloring')

The graph you presented contains multiple paths to the same element, C AND one path is longer than the other. This means that you could get the situation where 'C' is processed, but 'bc' isn't. and 'bc' isn't examined more than once, as it has to be accepted when 'c' is examined, but that doesn't happen in this case.

At least that's the conclusion I've drawn during the debugging so far. I hope to have a more clear answer soon.

Anyway, I've mailed Simon so I hope he responds soon, otherwise I've to make the changes and test whether they hold up in the unittests we have, though I don't like to rely on unit tests alone as they only reflect a small subset of the unlimited number of possible situations.

I think I see what you're saying about C and BC. Does this problem affect how you implemented graph flattening in GraphUtils?

About unit tests, at least with a case like this, couldn't code generation help come up with a more comprehensive set of unit tests? If you just ran the generator through cases with X entities with Y,Z, etc fields with A,B, etc relationships and unit test serialization with C,D,etc set of instances...? Just run the whole thing through random iterations overnight and maybe wake up the next morning with a crazy edge-case where your serialization algorithm doesn't work correctly. Maybe this is easier said than done

mikeg22 wrote:

Otis wrote:

mikeg22 wrote:

Strange that I am the first one to notice this...that fastserialization code has been in use for quite a while, it seems like.

Normal serialization is fine right now though, I can wait.

The problem is a specific edge case graph. When I analyzed the graph you presented in the debugger when stepping through the fast serializer code I recognized it from my early attempt to be clever in a depth first search based algo over the entity graph for saving: the algo uses two states for nodes: not visited and visited. But you need 3: not visited, visiting and visited. (the proper term for this is 'graph coloring')

The graph you presented contains multiple paths to the same element, C AND one path is longer than the other. This means that you could get the situation where 'C' is processed, but 'bc' isn't. and 'bc' isn't examined more than once, as it has to be accepted when 'c' is examined, but that doesn't happen in this case.

At least that's the conclusion I've drawn during the debugging so far. I hope to have a more clear answer soon.

Anyway, I've mailed Simon so I hope he responds soon, otherwise I've to make the changes and test whether they hold up in the unittests we have, though I don't like to rely on unit tests alone as they only reflect a small subset of the unlimited number of possible situations.

I think I see what you're saying about C and BC. Does this problem affect how you implemented graph flattening in GraphUtils?

No, that was fixed a long time ago. The fastserializer doesn't use the graph utils in this particular case. I think this is because the graph utils were added after the fast serialization code (v1) was written.

About unit tests, at least with a case like this, couldn't code generation help come up with a more comprehensive set of unit tests? If you just ran the generator through cases with X entities with Y,Z, etc fields with A,B, etc relationships and unit test serialization with C,D,etc set of instances...? Just run the whole thing through random iterations overnight and maybe wake up the next morning with a crazy edge-case where your serialization algorithm doesn't work correctly. Maybe this is easier said than done

heh You can generate unit-tests but mainly based on pre/post conditions. Bertrand Meyer from Eiffel (which does contains this) has written some papers about this and they used it to generate unittests for the eiffel code itself and with that they found some obscure bugs.

Other than that, it's not really doable as you can easily run into an infinite # of states to test. As with these graphs, it's doable to prove that things aren't going that well, however to see if the rest of the serializer code indeed doesn't add duplicates somewhere is another question. (i.e.: you then need the info what has been optimized. )

Hi All

Frans: Saw you sent me a private message but unable to access it at the moment. Can you send a copy to simon.hewitt AT bp.com

Its such a long time since I looked at this code!! I can't even remember what the definition of a 'referenced' entity is with regard to parent/child relationships. Or which way round Depending/Dependant entities are etc.

Off the top of my head, I think the ReferencedEntityMap class might be the key here. Is bc returned in GetReferencedEntities() or not?

I'm really stuck for time at the moment but I'll have a look and try and help.

Cheers Simon

My webmail has now come alive again so I'll read what you said and get back to you.

Cheers Simon

Simon, I'll mail you the sourcecode I have used to test it. I'll fix up the references a bit so you can drop in own builds of the runtime lib to step into the sourcecode. See if you can find the time to look into this, if not, I'll give it a go

Fix mailed to Frans for testing.

Cheers Simon

simmotech wrote:

Fix mailed to Frans for testing.

Cheers Simon

Received. Thanks for the time, Simon . Tests are running now. Will get back to you and this thread after the tests have been completed.

(edit) succeeds . I'll attach a new runtime build after the packaging has run.

Otis wrote:

simmotech wrote:

Fix mailed to Frans for testing.

Cheers Simon

Received. Thanks for the time, Simon . Tests are running now. Will get back to you and this thread after the tests have been completed.

(edit) succeeds . I'll attach a new runtime build after the packaging has run.

Awesome. I'll give it a shot and let you know if I have any problems.

Thanks Simon!

Otis wrote:

simmotech wrote:

Fix mailed to Frans for testing.

Cheers Simon

Received. Thanks for the time, Simon . Tests are running now. Will get back to you and this thread after the tests have been completed.

(edit) succeeds . I'll attach a new runtime build after the packaging has run.

Any chance you can put this in a new release with the runtime sourcecode?

mikeg22 wrote:

Otis wrote:

simmotech wrote:

Fix mailed to Frans for testing.

Cheers Simon

Received. Thanks for the time, Simon . Tests are running now. Will get back to you and this thread after the tests have been completed.

(edit) succeeds . I'll attach a new runtime build after the packaging has run.

Any chance you can put this in a new release with the runtime sourcecode?

Yes, but we've 1 other thing to fix first. If you really need the sourcecode now, I'll email you the sourcecode

Otis wrote:

mikeg22 wrote:

Otis wrote:

simmotech wrote:

Fix mailed to Frans for testing.

Cheers Simon

Received. Thanks for the time, Simon . Tests are running now. Will get back to you and this thread after the tests have been completed.

(edit) succeeds . I'll attach a new runtime build after the packaging has run.

Any chance you can put this in a new release with the runtime sourcecode?

Yes, but we've 1 other thing to fix first. If you really need the sourcecode now, I'll email you the sourcecode

No, its ok. I can wait.

Runtime libs are now available