ArraySeq dead code cleanup and ArraySeq_short gap filling

Description

The ArraySeq constructor and all methods retain support for primitive ArraySeq but that code has all been shunted into type-specific ArraySeq primitive array variants (ArraySeq_long, etc). The ArraySeq_short variant is currently missing.

Approach: Remove the vestigial primitive array code paths in ArraySeq and fields (ct and oa). This may provide a performance benefit but it has been hard to find a measurable impact.

The patch also fills in the missing ArraySeq_short variant. Before this patch, reduce on an array of primitive short would throw ClassCastException.

Patch: no-getComponentType--v002.patch
Screened by: Stuart Sierra

Environment

None

Activity

Show:
Alex Miller
January 11, 2014, 2:54 PM

Point to considered patch, this got lost at some point in the description.

Alex Miller
December 17, 2013, 5:40 PM

Removed performance angle and focus on clean-up and gap-filling (ArraySeq_short) aspects instead.

Rich Hickey
November 22, 2013, 7:44 PM

please add perf tests that demonstrate benefit

Brandon Bloom
October 8, 2013, 3:43 PM

Hey Alex: Thanks for following through on the vestiges removal.

Alex Miller
October 8, 2013, 4:25 AM

The existing ArraySeq class has support for different two modes - as an array of Objects and as an array of primitives. In the Object case, this.oa will be set to the array and this.ct will be set to the component type of the array. From running a bunch of code, this.ct is not always Object - it is easy to find cases of Class and many other cases as well.

However, any kind of non-primitive array will cause this.oa to be set and this prevents the calls to prepRet from being called with ct in every method where this is done. All primitive arrays are now being handled via the switch in ArraySeq.createFromObject.

The only thing prepRet is effectively doing is returning canonical Boolean objects. It is possible to create an ArraySeq on a Boolean[]. However, even in this case, Boolean[] is an instanceof Object[] and the object path is triggered.

Thus, I agree that prepRet is never being called from ArraySeq and this path can be removed, which also allows us to remove this.ct and importantly the array.getClass().getComponentType() check. This also allows us to go further though and remove the oa field entirely and all of the prepRet calls.

I have attached an updated patch that makes this change. I also found (based on some test failures) that the ArraySeq_short was inexplicably missing so I added that in as well.

ArraySeq could be made even cleaner with the addition of another variant that specifically handled the case of a null array (ArraySeq_null). I have no idea if that would be worth doing and I have not done that here.

Completed

Assignee

Unassigned

Reporter

Brandon Bloom

Labels

None

Approval

Ok

Patch

Code

Fix versions

Affects versions

Priority

Minor