IL: optimize attribute cluster reading

[auduchinok]

• Removes ImmutableArray builder creation, its array reversion, another array allocation and copying for sorted tables
• For unsorted tables it prevents the reader from going further through the table after a cluster of matching attributes is found

The second optimization may in theory break things if any compiler produced the attributes out of order, but AFAIK none of the popular compilers do it, and we have been using it for many years without ever seeing an issue. @dsyme @vzarytovskii What do you think about it?

[auduchinok]

This is ready.

[dsyme]

The second optimization may in theory break things if any compiler produced the attributes out of order, but AFAIK none of the popular compilers do it, and we have been using it for many years without ever seeing an issue. @dsyme @vzarytovskii What do you think about it?

Could you check if F# produces attributes out of order please, if multiple instances of the same attribute are given in source text out of order. Thanks

[auduchinok]

@dsyme do you mean checking if there's a case where attributes of one entity are mixed with attributes of another one? I thought the table is populated sequentially for each method, type, etc, i.e. nothing would write other things to it in the middle of generating attributes for a particular attribute owner.

[dsyme]

@auduchinok Ignore my comment, and I think your code is fine, but to double check could you please clarify what you mean by this:

The second optimization may in theory break things if any compiler produced the attributes out of order

[auduchinok]

@dsyme Consider this pseudo table that says which entity does each attribute belong to:

1, Type1
2, Type1
3, Type2
4, Type1

The previous code would find all three attributes for Type1 for unsorted tables and the current one would only find one of the two separated groups. It seems the popular compilers don't produce tables like this, and I've seen this optimization used in other places, so it's probably not too bad?

[dsyme]

@dsyme Consider this pseudo table that says which entity does each attribute belong to:

Sorry, just to be really clear, can you give headings for those columns? You mean like this?

Index     Parent      Attribute/Type/Constructor
1            Type1       SomeAttribute()
2            Type1       SomeAttribute()
3,           Type2       SomeAttribute()
4            Type1       SomeAttribute()

ECMA-335 says the CustomAttributes table must always be sorted by "Parent" so it's possible that the code not using a binary chop is never used.

[auduchinok]

Sorry, just to be really clear, can you give headings for those columns? You mean like this?

Yes, exactly, thanks.

ECMA-335 says the CustomAttributes table must always be sorted by "Parent" so it's possible that the code not using a binary chop is never used.

Oh, indeed. I guess the original code was meant to be more generic than only getting attributes. Then all is good.

[dsyme]

Oh, indeed. I guess the original code was meant to be more generic than only getting attributes. Then all is good.

Yup, Or else the binary chop was added later and the old code was left there to test correctness or in case we needed it