Cool.
Just to be sure I understand, using the FieldCollection replaces the Field[] in
every Row with a List<Field> and a Dictionary<string,Field>. If so, I worry
about memory consumption. We've had problems in the past blowing out memory
when linking or binding (I forget which) really large MSI packages.
Fortunately, it does seem we could keep the dynamic API without necessarily
using the List and Dictionary by looping through the fields. The perf of O(1)
provided by the Dictionary would be lost but at least the Field array is
usually pretty small. Does this sound right?
Finally, do you have some specific cases you can point to where creating
classes that wrap the generic collection implementations would improve the
codebase? We had (still have a few) wrappers that strongly typed the ancient
non-typed collections. However, many times "helper functions" were added to
those collections that did bad things to the code. So, if there are specific
cases where the code is cleaner, I'm all for wrapper classes but we need to
apply good design to the helper functions. <smile/>
_______________________________________________________________
FireGiant | Dedicated support for the WiX toolset |
http://www.firegiant.com/
From: Heath Stewart [mailto:hea...@outlook.com]
Sent: Monday, October 13, 2014 12:12 AM
To: WiX toolset developer mailing list
Subject: Re: [WiX-devs] ICE Breaker v0.0.1
I implemented the changes and analyzed the perf using a test that compiled a
document with sufficient parsing (not a large document, but that exercised the
changes to the code a lot) 1,000 times.
The differences were largely negligible, but I'm a little suspecting of them.
The comparison is attached from OneDrive below.
Basically, the baseline (no DLR) was 4.1s while just adding support for the DLR
was 4.9s (this is what I doubt). Using the DLR (i.e. dynamic binding) was only
5.4s.
The difference between the first set I have trouble believing. Because I ended
up creating a ReadOnlyKeyedCollection as a base for FieldCollection - which
gave index-based accessors as well as named index accessors - which actually
decreased the amount of enumerations made elsewhere in the code by having
Row.Fields return an array (which is an fxcop rule violation, though not a huge
deal in and of itself). The perf comparison seems to indicate that time
should've been less with the changes I made to support the DLR.
At any rate, with 1,000 compilations the different seems negligible. I also
don't think we should necessarily go through and replace all the index
references in the core with DLR binding. This wasn't my vision when I
implemented something like this for testing internally either. It was more to
make it easier for people to write tests or other tools on top of.
(Additionally, part of the reason I did this internally was to abstract the
index bases differences between MSI and WiX away from callers.)
See https://github.com/heaths/wix4/tree/dynamic_example for the changes -
primary the HEAD~1 commit. The HEAD commit was changes to the compiler to
actually use the DLR, but I got tired of doing all of them and so only changed
what I'd need to test - I also kept ParseMajorUpgradeElement as-is except to
still declare the 'row' as dynamic to show that you can still use the old
methods of access whether you used a typed variable or a dynamic variable.
Whatever the case, I think there's still value in a few of the changes I did in
the first commit, like the FieldCollection that makes it easy to reference
fields by name (regardless of the DLR, so something like row["Property"]
instead of row[0])and the ReadOnlyKeyedCollection<TKey, TItem> which emulates
IReadOnlyList<T> (v4.5) while supporting what KeyedCollection<TKey, TItem> does
while hiding all the modification methods. While I see you're getting rid of a
bunch of past collection classes, I'd urge you to consider using something
derived from their old generic equivalents since you can add methods /
properties to them that might be more friendly to use and decrease the amount
of LINQ/extension methods you have to call (which can get expensive) - all
while still supporting them should people want to use them. Just gives you more
flexibility for any future changes you might want to do without having to
refactor code for type changes (i.e. a FieldCollection will always be a
FieldCollection but we can change the underpinnings all we want).
Heath has a file to share with you on OneDrive. To view it, click the link
below.
[https://a.gfx.ms/ftype/v4/xlsx.png]WiX Dynamic Perf Analysis.xlsx
<https://onedrive.live.com/redir.aspx?cid=9415f61cbb1a8030&resid=9415F61CBB1A8030!135853&parId=9415F61CBB1A8030!8580&authkey=!AAcxzH_qumbQZn8&ithint=file%2cxlsx>
------------------------------------------------------------------------------
Meet PCI DSS 3.0 Compliance Requirements with EventLog Analyzer
Achieve PCI DSS 3.0 Compliant Status with Out-of-the-box PCI DSS Reports
Are you Audit-Ready for PCI DSS 3.0 Compliance? Download White paper
Comply to PCI DSS 3.0 Requirement 10 and 11.5 with EventLog Analyzer
http://p.sf.net/sfu/Zoho
_______________________________________________
WiX-devs mailing list
WiX-devs@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/wix-devs
![https://a.gfx.ms/ftype/v4/xlsx.png]WiX](https://a.gfx.ms/ftype/v4/xlsx.png]WiX){kind=link}