CharMatcher.retainFrom allocates a new CharMatcher each time it's called #1506
Description
Original issue created by cpovirk@google.com on 2013-08-14 at 07:10 PM
Possibly not a big deal, since the caller of retainFrom is necessarily accepting that he may allocate a whole new String, but possibly easily avoidable. Here's an internal discussion, reposted externally with light editing.
Seems to imply that:
for (String s : input) {
CharMatcher.WHITESPACE.retainFrom(s);
}
Will allocate input.size() new instances of NegatingMatcher.
Is it worth lazily caching (no synchronization as it's idempotent) or even
creating always given that matchers are likely to have long lifetimes and
high reuse?
I think it would be sensible for e.g.
CharMatcher.DIGIT.retainFrom(text) not to have to create a new
negation-of-digits matcher every time it's called, yes.
If the easiest way to achieve that is to have CharMatcher memoize the
result of negate(), then yes, that seems like a reasonable thing to
do; I don't think that anyone expects CharMatcher instances to be
particularly lightweight. (I wouldn't bother creating the negation
ahead of time, though.)
It looks like doing that might help in a few other areas, too; e.g.
common.net.MediaType.parse(), which has references to a few different
matchers used in different contexts, but which then calls .negate() at
the point of use, every time.
Alternatively you could take a look at making CharMatcher.retainFrom()
not require a negated matcher. From what I can see, most people who
call negate() are already keeping the reference anyway, sometimes
after calling precomputed() (which you wouldn't want to do in general,
I suspect.)