ericvana / protobuf-net

Automatically exported from code.google.com/p/protobuf-net
Other
0 stars 0 forks source link

v2 beta doing weird things? #176

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
Just got v2 beta but haven't used previous versions so I may be doing something 
wrong.

Have compiled protobug-net using default settings and used the Release version 
dll in another project to do some timings for your NWind sample classes/data.

If I leave AutoCompile on (or manually compile) then everything works as 
expected for round-tripping (I using Order[] only not the top level Database, 
not sure if this is relevant) - I serialize then deserialize and get exactly 
the same classes as expected and the byte[] created during serialization is 
exactly the size as the original (133,010 bytes)

However, if I set AutoCompile=false, then I noticed that serialization size was 
only 91,380 bytes. Further investigation showed that the Lines property was not 
serializing.

Flipping back to your solution, I noticed similar problems. For example the 
NWindPipeline Test was failing because 0 Lines were found instead of 2155.

I did try some tracing (rather complicated!) and found that in 
PropertyDecorator.Read, oldVal and newVal and value.Lines were the same object 
(an instance of EntitySet<DAL.Order>)and contained the expected OrderLine 
entities.

However, this line:
property.SetValue(value, newVal, null);
calls the Order.Lines setter which tries to set the Lines property but the 
Assign method ends up removing them!

So something somewhere should not have been trying to set the same value on the 
property; and something somewhere should not be clearing the list when it is 
being Assigned two items! Also I don't understand why it is working in some 
scenarios but not others.

Original issue reported on code.google.com by simon.he...@simmotech.co.uk on 29 May 2011 at 11:32

GoogleCodeExporter commented 9 years ago
Thank you for the detailed report. I will investigate.

Original comment by marc.gravell on 29 May 2011 at 3:00

GoogleCodeExporter commented 9 years ago
Wow; that is an insane change inside LINQ-to-SQL in 3.5.* (it is fixed in 4.0) 
- I will tweak to work around it. Annoyingly, IIRC I had to put this "set 
anyway" to fix another similar dodgy scenario - it'll be fun finding a version 
that works for both.

Original comment by marc.gravell on 30 May 2011 at 8:55

GoogleCodeExporter commented 9 years ago
I meant to say "insane bug inside LINQ-to-SQL"

basically - if you assign an EntitySet setter the existing value, it 
self-immolates.

Original comment by marc.gravell on 30 May 2011 at 8:56

GoogleCodeExporter commented 9 years ago
For info, I have this fixed locally but I want to fix a few other list oddities 
before commit

Original comment by marc.gravell on 30 May 2011 at 2:11

GoogleCodeExporter commented 9 years ago
Fixed r400

Original comment by marc.gravell on 30 May 2011 at 10:51

GoogleCodeExporter commented 9 years ago
The only way I can get OrderLines deserializing properly (even with r400) is 
ReleaseMode/Compile. Any DebugMode or ReleaseMode/No Compile ends up with no 
OrderLines at all.

Will investigate and get back to you.

Original comment by simon.he...@simmotech.co.uk on 31 May 2011 at 2:16

GoogleCodeExporter commented 9 years ago
Wanted to rule out it was anything I was doing in my project so I've reproduced 
the issue with a Unit Test in your project (under Examples\Issues).

If you run it as-is under Release Mode, it will pass. Uncomment the line and it 
will fail. Or change to Debug Mode and it will fail regardless of the commented 
line.

Original comment by simon.he...@simmotech.co.uk on 31 May 2011 at 2:35

Attachments:

GoogleCodeExporter commented 9 years ago

Original comment by marc.gravell on 31 May 2011 at 10:09

GoogleCodeExporter commented 9 years ago
Wow; this is my second crazy LINQ-to-SQL bug; the `set.Assign(set)` I have 
worked around already, but check this out for insanity: 
http://stackoverflow.com/questions/6194639/entityset-is-there-a-sane-reason-that
-ilist-add-doesnt-set-assigned

(I've also pinged some data folks at MS, to clarify whether this is 
known/expected)

I will work around this, but: it'll be tomorrow... very late here now ;p

Original comment by marc.gravell on 31 May 2011 at 11:25

GoogleCodeExporter commented 9 years ago
Fixed (and logged on connect)

Original comment by marc.gravell on 1 Jun 2011 at 5:48

GoogleCodeExporter commented 9 years ago
Not had a chance to try latest fix out yet but I can't see how it accounts for 
failing on three configuration combinations and only working on the fourth.

Original comment by simon.he...@simmotech.co.uk on 1 Jun 2011 at 11:51

GoogleCodeExporter commented 9 years ago
Simple; when *compiled*, there is a lot of static analysis done to decide the 
most appropriate way to add data. The typed Add(Foo) method is preferred, and 
used when available.

When it is running without *any* pre-compilation, it is using reflection, i.e. 
based around "object". The fastest way to add an item to a list when all you 
know is "object" is to check for support from the non-generic IList interface. 
If this is found, it can use that pretty easily. If not found (or, now, if 
explicitly told not to for EntitySet), it uses reflection against the same Add 
method that the *compiled* code would have used, but this (reflection invoke) 
is slower than any of the methods mentioned above.

Original comment by marc.gravell on 1 Jun 2011 at 11:59

GoogleCodeExporter commented 9 years ago
Right. I sort of guessed that the compile option was choosing the other Add 
method.

But what I still don't understand is why it failed under Debug regardless of 
the compile setting. (I just double-checked this by reverting ListDecorator to 
r400)

Original comment by simon.he...@simmotech.co.uk on 1 Jun 2011 at 1:20

GoogleCodeExporter commented 9 years ago
To help with testing/debugging, AutoCompile defaults to ***off*** in debug mode 
(and *on* in release-mode). That way I can toggle between debug/release and 
also test compiled mode. This is necessary as reflection is a necessary 
fallback in some cases, but in regular .NET you won't ever really see it.

It is not intended for users to use anything except the release version, so I 
am particularly grateful for spotting this oddity.

Original comment by marc.gravell on 1 Jun 2011 at 4:26