Specs slowed down alot

[IvanShamatov]

Thanks for your gem, but you'd probably should know that after you added Marcel gem, it slowed specs a lot. We use validations only in 2 places and every validation check took 5minutes locally and 12 minutes on GHActiions for each check. I believe the reason for that is that Marcel (when you don't pass name) opens the file. https://github.com/rails/marcel/blob/51a8f5535f927c1494bd6fc78abc3021aedf681b/lib/marcel/mime_type.rb#L64

version: 0.9.4

0.60988 seconds ./spec/models/model_spec.rb:15
0.54952 seconds ./spec/models/model_spec.rb:16

version 0.9.5

314.05 seconds ./spec/models/model_spec.rb:15
311.79 seconds ./spec/models/model_spec.rb:16

[igorkasyanchuk]

What options do we have? Can we add caching?

[IvanShamatov]

At least we can pass the name, so Marcel can guess mime type by name. Or do not use Marcel::TYPES only. You are the author, what would you recommend?

[randsina]

Not clear what's the validations cause this behaviour. Could you provide your test suite?

1. Marcel opens the file only if you pass the option pathname_or_io. It doesn't happen in this validation. https://github.com/igorkasyanchuk/active_storage_validations/blob/8cd2eadc7aa67359cbca51ef05a8f96037f8a39a/lib/active_storage_validations/content_type_validator.rb#L29
2. Marcel::TYPES provides content_type kind of image/png. It's required to handle reduction with symbols/strings like :png by originally.

[IvanShamatov]

Ok, guys, I'll try to create a clean example with no extra deps, and will post a repo here, thanks

[StefSchenkelaars]

It seems that the slowness is originating from Rails 6.1.4 (which actually switches the Marcel gem).

[joshmcarthur]

This slowness seems to be a scale issue, rather than a particular bottleneck. The content type matcher attaches a file for a given type to the field, and then checks there are no errors. Because it runs all the validations on the field to check validity, I think it also runs any other validations (like dimensions, which AFAIK analyses the image file)

The problem seems to be that it does this about Marcel::TYPES.keys times - 901 times based on my version of Marcel, causing a compound slowdown. I'm still digging to see why it does it this so many times.

[joshmcarthur]

Oh, so rejected_types defaults to Marcel::TYPES.keys - allowed_types, and this is used by both the allowing and rejecting qualifiers of the matcher.

I think the intention of this is to check both 'sides' of the validation - both that it allows the content types specified, and that it denies any other content type. The problem with this is that it needs to attach a tempfile ~ 900 times.

A temporary workaround for this issue is to EITHER specify an empty array for rejecting or allowing - whichever one is the inverse of what you're actually checking, OR specify both the allowing and rejecting matcher methods chained one after the other. For example, the following examples all runs in about 0.3 seconds for me, instead of 38 seconds each:

it { is_expected.to validate_content_type_of(:logo).allowing('image/png', 'image/jpeg').rejecting([]) }
it { is_expected.to validate_content_type_of(:logo).rejecting('application/pdf', 'image/bmp').allowing([]) }
it { is_expected.to validate_content_type_of(:logo).allowing('image/png', 'image/jpeg').rejecting('application/pdf', 'image/bmp') }

Unfortunately, I don't see a non-breaking solution for this. I think the intent was to say that if you specify an allowing argument, you'll get the inverse rejecting types without having to add another spec, but really the fix would be to not default the reject list to the full list of known mimetypes - so a way to fix this would be to default the rejecting list to an empty array - the same as the allowing list defaults to. But, there might be users of this gem that are utilising this behaviour and so the spec would actually stop implicitly checking for non-allowed types are rejected.

[rmu-wtag]

Without using rejecting, the spec becomes very slow, it took almost 10 min to pass the spec. How rejecting has been used for which the spec ran faster? Can anybody explain?

[sobrinho]

@igorkasyanchuk would you check #169 ?

[tagliala]

Hi, I can confirm that there is an improvement in 1.0.1 when using the allowing matcher

1.0.0: 2.57 seconds
1.0.1: 0.05458 seconds