WIP: Refactor metadata
Created by: myronmarston
Convert Metadata to use composition rather than inheritance.
- It's best to avoid subclassing core types like Hash.
- Remove runtime module extensions, since that blows MRI's method caches and has an affect on perf.
- Instantiation was a two part process:
.new
followed byprocess
. Conceptually, these belong together, so switch to using a single method for instantiation.
Two gotchas I discovered:
- The
include
matcher didn't initially work properly withMetadata
. It uses anis_a?(Hash)
check to know how to treat the object. To solve this, we overrideis_a?
to return true for Hash. -
dup
didn't work as expected.dup
is used to get a copy that can be modified without affecting the original; however,dup
was simply duping the wrapping object, and each retained a reference to the same wrapped hash, causing changes on one to show up on the other. I overridedup
andclone
to address this.
However...this seems to have a noticeable negative perf impact, in spite of the fact I expected it to have a positive perf impact (due to no longer using module extensions at run time). Here are 3 runs of the test suite against this branch:
➜ rspec-core git:(refactor_metadata) ✗ bin/rspec | grep Finished
Finished in 3.27 seconds
➜ rspec-core git:(refactor_metadata) ✗ bin/rspec | grep Finished
Finished in 3.02 seconds
➜ rspec-core git:(refactor_metadata) ✗ bin/rspec | grep Finished
Finished in 3.12 seconds
Compare that to master:
➜ rspec-core git:(master) ✗ bin/rspec | grep Finished
Finished in 2.94 seconds
➜ rspec-core git:(master) ✗ bin/rspec | grep Finished
Finished in 2.98 seconds
➜ rspec-core git:(master) ✗ bin/rspec | grep Finished
Finished in 3.05 seconds
I'm not sure why. This seems like a better design to me, but I don't want to merge it if it's going to have that much of an affect on perf. There may be a way to salvage it though.
Thoughts?
/cc @alindeman @JonRowe @soulcutter @samphippen