I want to see if I can simplify the 'require' process for the loading of this gem. Today we only 'require' a generated definition either when it is explicitly loaded or when the region is used for the first time. This means we have some weird 'requires' logic that is buried in our files.
It would be great if I could simply auto-require all generated definitions and then, when a region was loaded, I could simply call a method to 'merge' that into memory. It would also allow for a lot of other benefits, like allowing for methods to return all of the 'custom methods' and associating those custom methods only with that region. Today 'custom defined methods' are basically global, which sucks. It's just luck that we have not stepped on toes.
Today a user of the gem will require holidays. This loads the main holidays code but does not require or load any generated definitions.
When a user attempts to use the gem logic (i.e. between or on) then a parsing of the user input of the region takes place. Assuming it is a valid region we attempt to then require the generated definition. Inside of the generated definitions the last line calls Holidays.merge_defs(). This loads the actual definition data into our memory repositories.
What I propose is to modify the existing definition generation process to not add the 'merge_defs' method to the generated files.
Instead we will have the existing defined_regions and holidays_by_month methods and will be adding a custom_methods function. Then we will modify the existing lib/holidays/option/context/parse_options.rb file to no longer simply require a definition file. Instead, armed with the region we will be able to do something like this:
REGION_UPPERCASE = <calculate the region in the proper format based on user input>
Holidays.merge_defs(Holidays::REGION_UPPERCASE.defined_regions, Holidays::REGION_UPPERCASE.holidays_by_month)We will call the merge_defs to get the information into memory.
This will have a few immediate benefits:
- No more requiring files based on user input. We will only require files that we explicitly know about in our gem.
- We can modify
merge_defsto accept custom methods. This means that we can load those into our memory repositories as well and not simplyrequirethem into the global namespace as we do today. - While requiring all of the definitions at startup is going to have a performance impact it will be lessened because we will only load the regions actually into memory that the user wants.
Cons:
- As mentioned above I imagine loading all of the generated definitions will have a negative impact on performance.
I don't have muche experience with benchmarking like this so I wanted to keep this simple. Here is the simple file I wrote to test this:
require 'benchmark'
puts Benchmark.measure {
require 'holidays'
Holidays.between(
Date.civil(2016, 1, 1), Date.civil(2016, 12, 31), :us
)
}As you can see it simply loads the 'holidays' gem and then tries to use the between method for the :us region. I ran it with the current setup and then made changes to autoload and ran it again.
Feedback welcome if there is a better way to benchmark.
~/dev/holidays [issue-144?]
% be ruby benchmark.rb
0.020000 0.030000 0.050000 ( 0.052629)
0.020000 0.040000 0.060000 ( 0.058041)
0.030000 0.030000 0.060000 ( 0.049587)
0.020000 0.030000 0.050000 ( 0.050916)
0.020000 0.020000 0.040000 ( 0.045537) 0.070000 0.080000 0.150000 ( 0.150330)
0.070000 0.070000 0.140000 ( 0.138802)
0.070000 0.080000 0.150000 ( 0.159647)
0.070000 0.070000 0.140000 ( 0.146294)
0.070000 0.070000 0.140000 ( 0.145125)