Redmine

Thank you for posting the interesting improvement.

Your patch is very fast, but I think an application should not have codes which rely on the internal structure of third-party libraries. Your patch accesses mini_mime's instance variable '@db'.

I updated your patch not to manipulate an internal variable of mini_mime. It is 15% slower than your patch but 70% faster than the current Redmine::MimeType.of.

without patches:

               known    226.301k (± 5.0%) i/s -      1.131M in   5.010006s
             unknown    225.491k (± 4.6%) i/s -      1.131M in   5.024550s

replace-mime-types-with-mini_mime.diff: (faster):

               known    382.020k (± 6.2%) i/s -      1.923M in   5.052268s
             unknown    382.602k (± 6.6%) i/s -      1.908M in   5.008955s

mime_type.rb.patch by Pavel Rosický (fastest):

               known    431.349k (± 5.4%) i/s -      2.167M in   5.038918s
             unknown    432.307k (± 5.5%) i/s -      2.172M in   5.040046s

I updated my patch to preserve existing cache mechanism. Now it is fast also for unknown extensions.

               known    232.028k (± 4.9%) i/s -      1.170M in   5.052806s
             unknown    236.938k (± 4.6%) i/s -      1.199M in   5.071712s

Committed. Thank you for your contribution.

Pavel Rosický wrote:

please also remove
[...]

Committed. Thanks.

mini_mime 1.0.1 supports lookup_by_extension. We can remove the cache mechanism using known_types hash by making use of the new method. Attaching a patch to remove the hash.

Even without the cache mechanism, Redmine::MimeTypes.of is faster than the original code.

r17467 (before applying replace-mime-types-with-mini_mime-v3.diff):

Warming up --------------------------------------
               known    20.028k i/100ms
             unknown    12.677k i/100ms
Calculating -------------------------------------
               known    246.855k (± 6.0%) i/s -      1.242M in   5.049235s
             unknown    138.044k (± 5.9%) i/s -    697.235k in   5.068005s

with the patch 29359-remove-known-types-hash.patch:

Warming up --------------------------------------
               known    20.249k i/100ms
             unknown    11.787k i/100ms
Calculating -------------------------------------
               known    264.152k (± 8.0%) i/s -      1.316M in   5.015411s
             unknown    142.650k (± 8.6%) i/s -    719.007k in   5.076025s

Pavel Rosický wrote:

29359-remove-known-types-hash.patch - .downcase shouldn't be removed, both EXTENSIONS[] and lookup_by_extension expect lower-case

Fixed.

and the benchmark looks odd, in r17467 both known/unknown scenarios were cached in a class var, so I would expect simmilar performance

You are right. Maybe I pasted a wrong result. The following results are correct ones. Redmine::MimeType.of will be faster than r17467 in many cases.

r17467 (before switching to mini_mime):

found in array (file.patch)
                        164.476k (± 6.5%) i/s -    832.300k in   5.081748s
found in gem (file.zip)
                        167.251k (± 5.7%) i/s -    841.580k in   5.047872s
not found in gem (file.go)
                        168.700k (± 5.7%) i/s -    851.440k in   5.063132s

with the patch:

found in array (file.patch)
                        418.181k (± 6.0%) i/s -      2.099M in   5.036624s
found in gem (file.zip)
                        183.820k (± 6.9%) i/s -    923.232k in   5.047995s
not found in gem (file.go)
                        147.241k (± 6.7%) i/s -    734.046k in   5.007225s

benchmark script:

require 'benchmark/ips'
require 'mini_mime'
require './lib/redmine/mime_type'

Benchmark.ips do |x|
  x.report('found in array (file.patch)') { Redmine::MimeType.of('file.patch') }
  x.report('found in gem (file.zip)') { Redmine::MimeType.of('file.zip') }
  x.report('not found in gem (file.go)') { Redmine::MimeType.of('file.go') }
end

Committed 29359-remove-known-types-hash-v2.patch.