Add specs and fix memory leak (at least on Ruby 3.3+)

[paddor]

Added specs for #2108. These will run conditionally with either:

# spec_config.rb
ENV['SEQUEL_INTEGRATION_URL'] = 'sqlite://spec_sqlite.sqlite3?setup_regexp_function=true'

Or:

# spec_config.rb
ENV['SEQUEL_INTEGRATION_URL'] = 'sqlite://spec_sqlite.sqlite3?setup_regexp_function=cached'

On Ruby 3.3+ an ObjectSpace::WeakKeyMap is used to cache the Regexp objects without keeping them in memory forever.

[jeremyevans]

Thanks for the patch. Are there cases where you are using dynamic regexps in your app for SQLite filtering? I want to determine if this is an actual production issue.

Sequel doesn't use WeakMap or WeakKeyMap anywhere currently. From my experience with Ruby's own tests, writing deterministic tests that involve the garbage collector is challenging to get right.

The code you wrote for WeakKeyMap should work fine for Hash. How about a :regexp_function_cache_class option that sets the class to use (defaulting to Hash, but the user could specify WeakKeyMap)? That way we can avoid some of the complexity and don't need the garbage collector tests.

[paddor]

We do have internal applications that allow the user to filter by regexp. Memory leaks are not an issue there since we now use the Proc-variant of :setup_regexp_function and just clear the Hash periodically. So this is by no means something with priority. It just occured to me that Ruby 3.3's WeakKeyMap would be useful for this case.

Yeah, I can make those changes.

[paddor]

It now supports a :regexp_function_cache_class option. It looks up a constant if it's a String, so it can be specified in the connect URI.

The default is still WeakKeyMap, if available, with Hash as fallback. Let me know if this is too complex and you'd rather have no mention of WeakKeyMap at all.

I've removed the spec invoking GC.start.

[jeremyevans]

I would prefer to just use Hash. The documentation can mention the use of WeakKeyMap. Also, no conversion of strings to classes via Object.const_get, the user would need to specify the class as an option, it would not be available via the URI.

[paddor]

Okay, I've updated the PR.

[jeremyevans]

Thanks for working on this. I'll do some testing locally and should have this merged today.

[paddor]

You're welcome. No hurries.

[jeremyevans]

I squashed merged this at a6cc908, then made some changes at 433b937, making things a little more flexible, allowing for deterministic tests without using ObjectSpace. I also added a performance speedup on Ruby 2.4+.

[paddor]

Wow, looks great! Thank you.