Using reflection for GUIControlFactory (2 Viewers)

[NoMoDo]

JoeDalton, about the cost of regular assignments versus reflection:

Many people have different numbers on this, the simple reason being that they don't test the same thing. A call to FieldInfo.SetValue is only about 6 to 8 times more expensive. A call to Type.Invoke to do the same thing is significantly more expensive, as more reflection is involved.

If you look at the code, the test in the article you've mentioned doesn't compare regular assignments to SetValue, it compares them to GetType + reflecting on that type to find the fields we want to update + SetValue. In many cases, the first two can be amortized by using a simple cache.

 

[Mars Warrior]

Very nice results indeed. Although I can't second the actual startup gain (as I haven't tested it), removing so many unnessecary calls MUST make it faster, independent of the technique used...

If I look at my startup times, 15 seconds until splash screen is displayed, and another 25 seconds until home is displayed (40 sec total), a fairly large gain in startup time would be very much appreciated by me.

If I look at the risks (of introducing bugs...) I would think that this is very limited, since it only involves the startup. I can understand that if this would target some of the basis features of MP, Frodo would have some second thoughts about implementing/approving this change only days before he will join the sharks

So, independent which solution is "better" (I hate these difficult discussions...), so just looking at readability, speed and risks/changes involved for MP, what's the verdict for the implementation of this change???

 

[JoeDalton]

If I look at my startup times, 15 seconds until splash screen is displayed, and another 25 seconds until home is displayed (40 sec total), a fairly large gain in startup time would be very much appreciated by me.

Wow! That is a long time.
On my PC it takes about 0.5 seconds to display the splash screen
and 4.64 seconds to display home.

Are you running on .NET Framework 1.1 sp2?

Also, you can try to create a local user named ASPNET if it doesn't exist already, you can leave this account disabled.

 

[NoMoDo]

Patch uploaded to SourceForge.

My patch is up on sourceforge.

Note: I will only be available to discuss this patch up until Monday. After that, I will not be able to do any serious MP work for at least six months, and will only be able to come by the chat room on weekends, so please ask me any questions you might have about my patch before Monday.

@Frodo: I noticed that in many controls, there is a LOT of difficult-to-maintain duplication when one control is contained within the other (for example, the scrollbar in the listbox, etc etc...). This gets out of hand in FilmStrip, for example. I think that after merging my patch, it should be very easy to fix this. Controls that are contained within controls should be nested elements of the containing control's xml node, and they can each have their own name.

For Example:
<control>
.....<type>thumbnailpanel</type>
.....<main scrollbar>
...........<type>vscrollbar</type>
...........etc...
....</main scrollbar>
....<default thumbnail>
...........<type>image</type>


etc...

And the corresponding C# code would simply be:
[XmlSkinElement("main scollbar")] GUIvscrollBar m_scrollBar;
[XmlSkinElement("default thumbnail) GUIImage m_defaultThumbnail;

I think this will remove a few good dozens (more than a 100?) lines of needless duplication. And again, with my patch this should be extremely simple to introduce elegantly, but I'm not going to do this myself as I've tapped out on time I can spend on MP for a while (life gets in the way )

 

[Frodo]

Here it also takes about 5 seconds total before MP is fully running
I think 5 seconds from mouseclick to fully running is very acceptable.
Question is why it takes sooo long on your Mars Warrior's pc while its very fast on my & joe daltons pc??

(my pc is a simple 1.8GHz / 512MB / onboard geforce4)

Therefore i'm more interessted in performance enhancements to point 2.
That is to lower the cpu% when MP is up & running

Frodo

 

[NoMoDo]

It seemed longer , but when I got my stopwatch, it turned out MP takes 20 second to start first time after a fresh restart, and then 5 secs every extra time. So I guess whatever performance advantage (if there indeed is one) won't be felt... oh well .

BTW, is being able to switch skins without restarting MP a priority at all? We could do this with the new GUIControlFactory without destroying the controls. But I'm not sure how much the controls themselves will need to be changed or fixed to make it work.

 

[Frodo]

I just looked @ your patch
Besides any performance enhancements or not
this is a nice patch. It cleans up some messy code which is always a good thing.
One thing i missed was the new refactored GUIButton3PartControl.cs
Maybe you can include that one also?

I'll test this code and if its works ok,
i'm gonna add it 2 cvs

Frodo

 

[NoMoDo]

Frodo, please read my above post, I made an edit.

Thanks for your comments. I didn't re-do the 3partbutton because no-one seems to be using it? I can try to do it tonight, if I have time, but to be honest, I'm not sure if I'll have the time to do it as happens to most 18-19 y\o Israeli boys, I am being drafted .

Have you considered merging my previous patch as well?

P.S. I'll upload the "final" version of my Amazon\Buy\Walmart\Google art fetcher tonight.

 

[Frodo]

>BTW, is being able to switch skins without restarting MP a priority at all?

For me DVB-S/T/C has more priority,
but offcourse switch skins without restarting is a very nice feature
we would all like

btw i found 1 BIG bug in your code
All the scaling code is gone???
We need this because the skin is designed for 720x576, but
when you run MP in 1024x768 then all controls,images,...
need to be resized so everything looks nice in 1024x768
I think you simply threw away this code??

edit: it seems to be there, but not working correctly
Look @ the filmstrip for example

Frodo

 

[NoMoDo]

No, of course not, I simply moved it to the GUIControl class, which seemed more appropriate, as every control does its own scaling (and most of them do it differently.

See in GUIControl.cs, there is a virtual method called ScaleToScreenResolution(). Every control that needs to do some further scaling (beyond fitting the control's rect) overrides this method. The method is called from GUIControlFactory::Create.

EDIT: hold on, I'll try and run MP in different resolutions.