Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Version 2.0 #20

Closed
WojciechNagorski opened this issue Dec 6, 2016 · 68 comments
Closed

Version 2.0 #20

WojciechNagorski opened this issue Dec 6, 2016 · 68 comments
Labels
question Further information is requested
Milestone

Comments

@WojciechNagorski
Copy link
Member

Topics to version 2.0:

@WojciechNagorski
Copy link
Member Author

I wanted to do migration for inheritance. But I have a problem.
Currently in xml we have:

and e.g.:
class A : B,
class B : C

and if we have migration for A, B and C e.g.:
A has 2 migration - ver="2"
B has 1 migration - ver="1"
C has 1 migration - ver="1"

and of course user can add other migration for any type in the future.
I don't have idea how xml should look for this situation in case we want run migration from base type in current type. And what order to run migrations if user can add migration migration in order e.g. C,A,B,A. And we should deserialize all old xml.

Do you have any idea or do you think it is wrong way?

Now, user have to create migrations for all type and he have to call base migration in subclass manually. Maybe it is ok and it not need changes?

@Mike-E-angelo
Copy link
Member

Are you familiar with EntityFramework, by chance? I think the way out here is to create an IMigrationPlan object that has a set of steps (or commands/instructions) that are built and then executed. So in the case above, an IMigrationPlanBuilder would walk the hierarchy of a given type and create a set of IMigration objects, which would then be executed by the IMigrationPlan.

BTW, I don't think EntityFramework uses this strategy but it made me think of it FWIW. :)

@WojciechNagorski
Copy link
Member Author

I don't understand you. I think you didn't wonder enough about what I wrote or I didn't write understandable.

The idea is, ExtendedXmlSerializer could read any old XML. And If you have hierarchy of classes like this:
class A inherits from B
class B inherits from C

And in the past the classes have changed and you have old XML from each period time. E.g.

  1. In 2000 year you remove property from C
  2. In 2001 year you renamed property form A
  3. In 2002 year you change type of property from B
  4. In 2003 year you changed property from A.

E.g. We have only xml from A class.

Currently you have to write migrations for each changes for class A. If you have another class Z that inherits from B you have to create migrations with changes from point 1,2,3 too. In new configuration I would like to create migrations for base class, that will be run for subclass. But if you think about all old xml you don't know which migrations you should run for xml. Becouse in xml you have only version of class A. E.g In case xml from point 3. You still have ver="1".

I think that running migration from base class in subclass is wrong way.
I had idea that you could be create configuration for this situation:

ExtendedXmlSerializer.CreateConfig(cfg=>{
cfg.ConfigType<C>().AddMigration(ClassCMigration);
cfg.ConfigType<B>().AddMigration(ClassBMigration);
cfg.ConfigType<A>().AddMigration(ClassAFirstMigration).AddMigration(ClassASecondMigration);
});

And for other options:

cfg.ConfigType<OtherClass>()
.Property(p=>p.Id).ObjectReference().AsAttribute()
.Property(p=>p.Password).Encrypt()
.Property(p=>p.UserName).ChangeName("User").Order(1);

@Mike-E-angelo
Copy link
Member

Ahhh yes I have toooo many problems floating in my head right now and I did not grasp this one, doh.

Let me focus on getting you the PR and then we can circle back around to this one. I like how you're thinking though. :)

@WojciechNagorski
Copy link
Member Author

I've added test for v2:

Type=XmlSerializerTest  Mode=Throughput

                            Method |     Median |    StdDev |
---------------------------------- |----------- |---------- |
   SerializationClassWithPrimitive | 51.5149 us | 0.9199 us |
 DeserializationClassWithPrimitive | 80.0274 us | 1.2360 us |

Type=ExtendedXmlSerializerV2Test  Mode=Throughput

                            Method |      Median |     StdDev |
---------------------------------- |------------ |----------- |
   SerializationClassWithPrimitive | 900.8198 us | 26.4454 us |
 DeserializationClassWithPrimitive |  42.5747 us |  0.5083 us |
 
Type=ExtendedXmlSerializerTest  Mode=Throughput

                            Method |      Median |     StdDev |
---------------------------------- |------------ |----------- |
   SerializationClassWithPrimitive | 937.6775 us | 18.8019 us |
 DeserializationClassWithPrimitive | 122.2478 us |  2.2484 us |

These results are not bad but they are not good enough.

@WojciechNagorski
Copy link
Member Author

I think that your solution is too big for this small project. But it is good start.

Did you read this book book https://www.amazon.com/Pragmatic-Programmer-Journeyman-Master/dp/020161622X ? It could be good position for you.

Anyway, I want to come back to configuration.
W think that profiles are good for us but for users they could be to hard.
We should create similar configuration like in automapper.
You will be able to create static or instance configuration:

ExtendedXmlSerializer.Initialize(cfg => cfg. [...] );
//or
var config = new ExtendedXmlSerializerConfiguration(cfg => cfg.[...]);

Then you will be able to create serializer:

//Without configuratino
var serializer = new ExtendedXmlSerializer();
// with configuration
var serializer = new ExtendedXmlSerializer(config);
// or from configuration
var serializer= config.CreateSerializer();

In configuration you will be able to configure all settings:

for example:

ExtendedXmlSerializer.Initialize(cfg =>{
cfg.ConfigType<OtherClass>()
.Property(p=>p.Id).ObjectReference().AsAttribute()
.Property(p=>p.Password).Encrypt()
.Property(p=>p.UserName).ChangeName("User").Order(1)
.AddExtension(AttachedPropertyToOtherClass);

cfg.UseAutoProperties();
cfg.UseNamespaces();
cfg.AddExtension(AttachedPropertyToAllTypeExtension);
});

What do you think about it?

@Mike-E-angelo
Copy link
Member

Mike-E-angelo commented Dec 12, 2016

I think that your solution is too big for this small project

I completely agree. This has been a good experience for me. In that have I have not been very pragmatic in my approach here, haha. Thanks for that book. I bought it!

These results are not bad

You are far too kind! Those results are terrible haha. I basically took your awesome performance made nearly 20 times slower That is simply unacceptable! Not to mention incredibly embarrassing. 😛

I did do some further analysis over the weekend, and I was able to get a barebones test to about 400us, but it was very bare without any of the (bug-fixing) features I wanted to add.

So, I feel that I have taken the wrong approach here and have gone down the wrong path. However, I do have another idea I want to try out. If I can get it within 250us, then I think that would be worth exploring. Otherwise, I might have to bow out and cut my losses here. :(

What do you think about it?

Very awesome. :) I approve. That would be a nice way to add extensions and such, too. My only concern is using a static entry point. Make sure that this is used for applications and not testing as this will break tests (two tests inadvertently using the same configuration).

I've added this as #22

@WojciechNagorski
Copy link
Member Author

Are you working still on version 2 or you give up ?

@Mike-E-angelo
Copy link
Member

LOL no I am working on it! I have totally rewrote it and have got caught up in performance. It is much simpler now. However, not all the tests are passing yet. Once I get them passing I will send another PR your way. Hopefully today or tomorrow. This is the serializer still not deserializer. So the way I see it:

Taking a little longer than I had hoped, as it always does. I should be done by next week. Is that OK?

@WojciechNagorski
Copy link
Member Author

That sounds amazing! Thank!

I don't remember how fast was version 1.5.0, but 71 it is very fast.

After these points we can publish version 2.0. Of course I will add information about your effort in readme ;)

@Mike-E-angelo
Copy link
Member

Cooooool. :) Yeah, I am full throttle on this. I guess I have an interest in serialization, haha. Plus there are new things I am learning such as .NET Core and Benchmark.NET, so it's been very valuable!

Speaking of Benchmark,NET, I did get wrapped up this weekend with a mysterious issue that I couldn't seem to figure out. I have opened up an issue on Benchmark's repo, if you're interested:
dotnet/BenchmarkDotNet#330

BTW, the v1.x serializer was getting 76-74us/op on my machine.

@WojciechNagorski
Copy link
Member Author

Thank for info! I'll check it.

@WojciechNagorski
Copy link
Member Author

I saw your code. It looks much better and it has good performance! Great job!

In new year I will join to you to work on version 2.0. But now I have a holiday.
Merry Christmas & happy new year!
God bless you!

@Mike-E-angelo
Copy link
Member

Hey, thanks! Much appreciated! I am actually almost done with getting the tests passing. I should have that done today -- it's taken much much longer than anticipated! But once I have this checked in, it will be a much more simplified 2.0 and faster.

Unfortunately, the ~71us was before adding the type="<type>", so now performance is around ~79us on my machine, which is still pretty good. I am hoping that the new ConfigurationAPI will allow for new ways to make it faster eventually.

So next week I will be working on that ConfigurationAPI and hopefully when you can start working on this project again it will be done for you and I can finally start working on that deserializer. 👼 😄

Happy Holidays to you as well, @wojtpl2! I have appreciated the opportunity to assist you here. 👍

@WojciechNagorski
Copy link
Member Author

WojciechNagorski commented Dec 23, 2016

Great job!
I have slower computer ;)

.net 4.5

Type=XmlSerializerTest  Mode=Throughput

                            Method |       Mean |    StdDev |   Gen 0 | Allocated |
---------------------------------- |----------- |---------- |-------- |---------- |
   SerializationClassWithPrimitive | 66.3622 us | 0.7910 us | 24.3164 |  56.85 kB |
 DeserializationClassWithPrimitive | 72.4928 us | 0.9108 us |  8.3333 |  22.91 kB |

Type=ExtendedXmlSerializerV2Test  Mode=Throughput

                            Method |        Mean |    StdDev |   Gen 0 | Allocated |
---------------------------------- |------------ |---------- |-------- |---------- |
   SerializationClassWithPrimitive |  91.2706 us | 1.6230 us | 27.3763 |  62.75 kB |
 DeserializationClassWithPrimitive | 104.2177 us | 1.4177 us | 15.0716 |  42.92 kB |
 
Type=ExtendedXmlSerializerTest  Mode=Throughput


                            Method |        Mean |    StdDev |   Gen 0 | Allocated |
---------------------------------- |------------ |---------- |-------- |---------- |
   SerializationClassWithPrimitive |  88.6446 us | 1.5373 us | 21.3379 |  50.25 kB |
 DeserializationClassWithPrimitive | 105.0393 us | 0.8864 us | 15.0716 |  42.92 kB |
 
 
 
.net core

Type=XmlSerializerTest  Mode=Throughput

                            Method |     Median |    StdDev |
---------------------------------- |----------- |---------- |
   SerializationClassWithPrimitive | 92.0979 us | 2.5228 us |
 DeserializationClassWithPrimitive | 72.4231 us | 0.9267 us |

Type=ExtendedXmlSerializerV2Test  Mode=Throughput

                            Method |      Median |    StdDev |
---------------------------------- |------------ |---------- |
   SerializationClassWithPrimitive | 127.0124 us | 1.4236 us |
 DeserializationClassWithPrimitive | 104.3320 us | 1.9484 us |
 
Type=ExtendedXmlSerializerTest  Mode=Throughput
                            Method |      Median |    StdDev |
---------------------------------- |------------ |---------- |
   SerializationClassWithPrimitive | 125.0962 us | 1.8916 us |
 DeserializationClassWithPrimitive | 105.0153 us | 3.9010 us |

@Mike-E-angelo
Copy link
Member

Awesome! That's great, indeed. So basically maintaining v1 speed with a little overage. A couple thoughts:

  1. Looks like .NETCore compiles and works? That was something I thought of last night, but forgot to check! Good to see that it does work.
  2. Right now the current emitter used for ExtendedXmlSerializerV2Test is super basic and doesn't do recursion checking. :( When you use XmlExtendedSerializer.SerializationToolsFactory, a new emitter is employed that does do recursion checking. I hope to address this with the ConfigurationAPI.
  3. Speaking of SerializationToolsFactory. I was thinking we should deprecate this and send people to the new ConfigurationAPI instead. Is that OK with you?

Also, kind of weird that serializer is faster in .NET framework, but slower in .NET Core. heh. 😮

@WojciechNagorski
Copy link
Member Author

  1. yes it works.
  2. I think in version 2 we should remove old SerializationToolsFactory and create only new configurationAPI.

@Mike-E-angelo
Copy link
Member

Coooooooool. On further thought, I think it's better to get the Deserializer done first, as that involves less work (or should involve less work) and will provide less risk to build further features upon. I will also see if I can attend to #14 as well (should also support ImmutableArray too!)

@WojciechNagorski
Copy link
Member Author

What have you been up to? Have you worked on ExtendedXmlSerializer? Have you done something?

@Mike-E-angelo
Copy link
Member

Hey! Happy New Year. :) Yeah... I have worked on something but haven't finished it, unfortunately. Last week was not nearly as productive as I had hoped for, and it culminated in helping the parents with their home networking that took far much longer than it should have.

In any case, I probably have a few more days on getting you a deserializer, unfortunately. Are you able to wait until Wednesday?

@WojciechNagorski
Copy link
Member Author

Thank! Happy New Year for you too!
Yes of course. We don't have any deadline. I'm doing this project after hours just for fun and learning. Nothing by force. I hope you too ;) I just need information that you are still working or you already gave up. If you need more time and you want still working for this project - I'm glad to hear that.

@Mike-E-angelo
Copy link
Member

Haha yeah no worries, just making sure! That's my problem is once I commit I feel obligated. I also want to be sure there are no pressing expectations on your side, either. So, OK then. I will check in on Wednesday and let you know. 👍

@Mike-E-angelo
Copy link
Member

Heya... I wanted to check in. I have made some progress but it is taking a little longer than expected. I am thinking at this point I will have a new PR for the serializer/deserializer for you if not over the weekend then Monday at the very latest. I think I have finally landed on a design I am happy with, but I will not know until I am done and all the tests are passing haha. It looks promising, though.

@WojciechNagorski
Copy link
Member Author

No worries. I really wonder what you've done.

@Mike-E-angelo
Copy link
Member

Haha nothing too impressive. Just making it WORK, for starters. :P I had some invalid assumptions with the serializer that didn't work so well with the deserializer, so things were inconsistent. And messy. :P

@WojciechNagorski
Copy link
Member Author

Do you can push your changes to your repo or you want wait for the end of you work? I have time and I can read your code today.

@Mike-E-angelo
Copy link
Member

Yikes, yeah when I get it to a good point I will do that. I sort of got it into a funky state right now.

I will try to give you some insight, however. I was using INode and IEmitter and IWriter which turns out was WAY too abstract. So what I have done is simply make a converter, which is a reader and a writer (with first-hand knowledge of Xml namespace components):

public interface IConverter : ISpecification<TypeInfo>, IWriter, IReader {}

public interface IWriter
{
    void Write(XmlWriter writer, object instance);
}

public interface IReader
{
    object Read(XElement element);
}

So, simpler and more aligned with how you did things in v1.0. :)

@Mike-E-angelo
Copy link
Member

Also, I have gotten rid of the idea of "primitives" versus "objects". Now there are simply converters that are used for a particular type. This will make items such as #14 easier to implement, as well as implementing future types (such as ImmutableArray etc).

@WojciechNagorski
Copy link
Member Author

Your changes are ok. And I'm happy with your fast work. I don't have enough time to finish this project on my own. I think that this project has potential. I would be sad if it ended in failure.

I asked because I didn't see that in ExtendedXmlSerialization.Legacy you create old file ExtendedXmlSerializerConfig.cs with old functions. Sorry my fault. Now I understand.

I will do profiles (or modules) quickly because it is easy for me. In this case maybe I wait for your changes. When you use configuration in v2, I'll be able to do this.

What do you think?

@Mike-E-angelo
Copy link
Member

Ahhh yes, I would definitely wait for Configuration API integration work first if possible. I am cranking along here, so things are happening much faster. I should have attributes (#6) and collection properties (#7 ) done this weekend, and then all the extensions done next week. Then Configuration API to make it all work nicely. Then I can finally hand this off to you. :)

@Mike-E-angelo
Copy link
Member

Looks like VS2017 is set to be released on March 7th:
https://blogs.msdn.microsoft.com/visualstudio/2017/02/09/visual-studio-2017-launch-event-and-20th-anniversary

It would be cool (and I think very doable) to launch v2 around that time, we could even promote it as a VS2017-based project. After we upgrade to it, of course. :)

@Mike-E-angelo
Copy link
Member

Here's a really nice example of a quick site we could put together:
http://www.thepollyproject.org/

I just hate signing up for more work. ;)

@WojciechNagorski
Copy link
Member Author

I've added information about v2 in article on CodeProject.

@WojciechNagorski
Copy link
Member Author

7th March could be great date. I have birthday in half of March. 😄
VS2017 it is good idea. Do you know if project will be have new structure or configuration in VS2017? Now I cannot to install new version VS. If I could still use VS2015 you are allowed to upgrading.

@Mike-E-angelo
Copy link
Member

Cool! Hopefully we can make this work out in time. And yes, the entire project structure will have to be migrated to 2017. project.json is no longer and has returned to .csproj. This will be a good opportunity to try all the new bits. We might want to try a little after 7th as it will take some time to adjust. So maybe on your birthday? 😄 Well, at the latest, at least.

@WojciechNagorski
Copy link
Member Author

no comment for csproj 😞 I think that csproj is not bad but what for I learned project.json. Doesn't matter.

@Mike-E-angelo
Copy link
Member

Hah, well I'll comment on it: it's a total disaster! 😛 MSFT has done an amazing job with .NET and imperative code, but declarative data has not gotten the same treatment, and so here we are. I wish everything was Xaml, but alas I am but in the minority.

I almost want to make EXS a Xaml parser still. :P I've put enough time into this already that I should! Haha... maybe v3 or something.

@WojciechNagorski
Copy link
Member Author

That great!

I've seen the page Polly web. This page was created in Ghost. Have you ever used it.
It is great page https://www.staticgen.com/. On this page you can see comparison of static web generators. I think that http://gohugo.io/ or http://jekyllrb.com could be useful for us.
Then we can host this page as github pages. What do you think ?

@Mike-E-angelo
Copy link
Member

Wow! that's interesting. That definitely makes it easier than I was anticipating. I like the look of Jekyll better, but I am not sure. There are so many to choose from! Have you had experience in any of these? My preference would be to go with something in .NET "just in case" but I am definitely open to alternatives. Whatever is easiest. :)

@WojciechNagorski
Copy link
Member Author

WojciechNagorski commented Feb 14, 2017 via email

@Mike-E-angelo
Copy link
Member

Yeah, definitely don't want to create a custom solution. I was thinking "just in case", but that is not a good way of thinking of things. 😮

@WojciechNagorski
Copy link
Member Author

How's everything coming along for v2? Do you need help?

@Mike-E-angelo
Copy link
Member

Hah! Probably a question I will never answer "yes" to. 👼 Things have gone a little slower than expected, but I am finally putting some features together. I hope to have #6 and collection properties done this weekend, and then start on the extensions (references, encryption, custom xml, and migrations) next week.

If anything, I would like help on that website. Is that something you could probably assist with doing?

@Mike-E-angelo
Copy link
Member

Heya... heads up. Letting you know that with my latest commit that IPropertyEncryption is going away. Currently in ExtensionModel there is an IEncryption that is used for encryption. PropertyEncryption implies only for properties, whereas the Encryption can be used for anything.

Hope that makes sense. I wanted to mention it here so you don't think I am deleting your work. I feel bad enough as it is with all the changes I've made... even if they are turning out pretty well now, finally. :)

Please let me know if this will be a problem.

@Mike-E-angelo
Copy link
Member

Mike-E-angelo commented Mar 15, 2017

Btw, have you seen VS2017 yet? It is currently clocking 7800 (!) problems. Last week it was ~6950 when I first saw it after it was launched:
https://developercommunity.visualstudio.com/spaces/8/index.html

So, it might be best to wait a bit before using or upgrading to it. 😉 They might have an update for //build and/or Redstone 2 (Windows Creator Update).

In any case, I am thinking upgrading to VS2017 should be the very very last thing we do before launching v2 LOL.

@WojciechNagorski
Copy link
Member Author

In my case it can be tricky to have VS 2017 and new resharper. We must have web page if we want use free Resharper. I will submit our project to https://www.jetbrains.com/buy/opensource/?product=resharper

@Mike-E-angelo
Copy link
Member

Don't forget OzCode, as well! :D
http://o.oz-code.com/open-source

@WojciechNagorski
Copy link
Member Author

ok.

@Mike-E-angelo
Copy link
Member

I am going to close this issue now as everything is completed from a v2 perspective. The only thing not implemented in the original list yet is #14, which I will be attending to sometime soon.

@WojciechNagorski
Copy link
Member Author

Awesome! So many changes! Thank! I'll try read this today and try v2.0.

@Mike-E-angelo
Copy link
Member

Cool. Hopefully it works OK for you. :)

@Mike-E-angelo Mike-E-angelo added this to the 2.1.x milestone Oct 26, 2018
@Mike-E-angelo Mike-E-angelo added question Further information is requested and removed discussion labels Nov 21, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

No branches or pull requests

2 participants