Replace ArrayList in InputLanguageSource with fixed-size array, reduce allocations

[h3xds1nz]

Description

Replaces ArrayList with a CultureInfo[] of 1 item downcasted to IEnumerable to improve performance of retrieval and decrease allocations caused by allocating an ArrayList and adding an item into it.

Method	Mean [ns]	Error [ns]	StdDev [ns]	Code Size [B]	Gen0	Allocated
ArrayList	13.167 ns	0.2586 ns	0.3362 ns	488 B	0.0053	88 B
GenericList	8.222 ns	0.1907 ns	0.1690 ns	1,261 B	0.0038	64 B
PR__EDIT	4.229 ns	0.1225 ns	0.2864 ns	54 B	0.0019	32 B

Customer Impact

Improved performance, less allocations.

Regression

No.

Testing

Local build, CI.

Risk

As outlined in the discussion with @miloush, this is a change to a public type under the hood. If someone was relying on an implementation detail, that this was an ArrayList, it would break. Do note that is nowhere documented this is an ArrayList.

Documentation clearly states for InputLanguageList property (nobody should assume otherwise):
When implemented by a class, this property should return an object that implements the IEnumerable interface, and that supports enumerating a collection of CultureInfo objects, each representing a supported input language.

Microsoft Reviewers: Open in CodeFlow

[miloush]

I will just point out that this value is returned by public API via InputLanguageManager.AvailableInputLanguages. After this change, the type of the returned value will be changed, but only for the case where single input language installed. The list is also generated anew on every call and so "belongs" to the caller, who can be manipulating it. If you are playing nice and you want to clone the list first, well, the generic list, unlike ArrayList, does not implement ICloneable.

On the positive side, the ownership is not documented, the public contract is IEnumerable only and there does not seem to be a single project on GitHub using this API. So it's likely fine.

[h3xds1nz]

You're returning an interface for a reason, to interface with it. That is the contract that was given.

Relying on an implementation detail such as this is the same as using reflection on private types in a foreign assembly.

Note aside: Don't worry about the ArrayList in InputProcessorProfiles, its demise is coming.

[miloush]

Why List rather than an array then?

this is the same as using reflection on private types in a foreign assembly

Not quite as all the involved types are public and you were able to do this even in partial trust. In the sense of "relying on implementation detail", sure. But so is relying on the fact that the enumerable contains CultureInfos, yet I am pretty sure that would be slightly more concerning thing to change.

As I said I am happy with this one going ahead, I just wouldn't necessarily call it "None" risk.

Don't worry about the ArrayList in InputProcessorProfiles, its demise is coming.

Never had doubts about that. :)

[h3xds1nz]

Why List rather than an array then?

Now that is a very good question that I do not have an answer to. @siagupta0202 Would it be too late to change it?

Not quite as all the involved types are public and you were able to do this even in partial trust. In the sense of "relying on implementation detail", sure. But so is relying on the fact that the enumerable contains CultureInfos, yet I am pretty sure that would be slightly more concerning thing to change.

When implemented by a class, this property should return an object that implements the IEnumerable interface, and that supports enumerating a collection of CultureInfo objects, each representing a supported input language.

We can refer to the wording, relying on CultureInfo is documented and so that the returned object that shall implement only IEnumerable.

As I said I am happy with this one going ahead, I just wouldn't necessarily call it "None" risk.

Fair point. Could have been outlined better from my side :)

Never had doubts about that. :)

Wouldn't believe you if you said you did, haha.

[miloush]

Ah good idea checking the interface documentation, I missed that. The one for AvailableInputLanguages could certainly use some tightening (since the relevance/existence of the interface is not clear).

[h3xds1nz]

Yeah, agreed.

[siagupta0202]

Now that is a very good question that I do not have an answer to. @siagupta0202 Would it be too late to change it?

@h3xds1nz it's not too late, you can still make the changes.

[h3xds1nz]

@siagupta0202 Thanks, will push it shortly.

[harshit7962]

@h3xds1nz Thank you for your contribution.

[h3xds1nz]

@harshit7962 Thanks for including it.