Closed cy6erGn0m closed 6 years ago
I believe Guava is currently only used for the Caching functionality. While it would technically be possible to make it optional, working without a Template Cache would cause a significant performance hit.
Perhaps it should be some simple default cache and ability to customize cache service so user can include any cache. But including guava in general purpose library is definitely a bad idea
It is correct that guava is ONLY used for it's caching purposes and I've been careful to make sure it didn't creep it's way into other parts of the application. Implementing reliable, efficient, and highly customizable data structures to be used for caching is hard so I decided to delegate it to a third party library.
I'm open to the idea of replacing guava with something else but I don't want the burden of maintaining a complicated custom-built cache service and it's important that Pebble provide's a reasonable out-of-the-box default.
I'm reluctant to even create a "simple cache service" because before you know it everyone is asking for:
Have you considered using maven-shade-plugin and its relocation feature to "hide" Guava? I don't know if it's a good idea, I'm just asking...
I guess it would need a couple of unit tests to make sure this relocation doesn't break anything though. For example, if Guava dynamically loads some classes using their names, relocation may fail.
I did not know about that feature; I'll have to look into it. Thanks!
I don't think it is worth it for Pebble to be yet another library that bundles its own version of Guava.
Many bigger libraries already do that if they can't get away without it, but I don't think it's justified here.
Bundling Guava has two major disadvantages: one in build time, as the size of the JAR is incresed from 100-something-kb to over 2MB (the current size of Guava), and another in runtime, as you're causing what's effectively the same set of classes to be loaded multiple times.
If removing the explicit Guava dependency is that important, I would suggest either abstract the Cache, or do without it completely (by falling back to a simple ConcurrentHashMap
implementation if Guava is not in the classpath).
Another alternative to Guava (although only available for Java 8) could be caffeine.
It provides a Cache API that is very similar to the one provided by Guava, and builds on its implementation (I believe the project to be maintained by the same Googlers that created Guava in the first place).
Yeah, you make some good points. It doesn't sound like it's worth it to bundle guava. That caffeine library does sound promising and I am wanting to switch to Java 8 at some point this year.
I have to say that I really like the fact that Pebble works with Java 7... I'm launching a new web framework soon and I plan on making Pebble the main templating engine. But I want the framework to be compatible with Java7, at least for the first version. In general, I think more people will be able to use Pebble if you stay compatible with Java7 a little longer... But if you have real good reasons to "upgrade", I understand too!
At this point in time I don't have a good reason to upgrade to java 8, it's just been something on the back of my mind, so your feedback on the matter is helpful. I'll keep that in mind, thanks.
Not true with relocation...
I might have understood it wrong, but doesn't relocation mean that Guava will be instead packaged as com.mbosecke.com.google.guava
?
Because if that's the case, there are already several libraries who do that as well. For instance, if Pebble did that, and I bundled it in an application with both Jersey and Guava, I'd have 3 similar versions of Guava's classes (one rebundled in Pebble, one rebundled in Jersey, and the actual Guava one).
@jcarvalho Sorry about that, I updated my comment before reading your reply. I understand what you mean and you're right.
Those of us on Google App Engine are stuck with Java7 for now - they're working on Java8, but there's no announced timeline. That said, Pebble v2 doesn't work on GAE, so I guess we're stuck with 1.6 (or a fork) anyways.
Personally I think the right answer to this issue is to keep using Guava as-is and update to the latest with each release. The solution to jar hell is to keep moving forwards. Google manages to tolerate Guava's degree of binary compatibility and their codebase is certainly bigger than any of ours...
How I'd do it:
A PebbleCache interface should be introduced, its use shall be optional (off by default), and external package shall be provided with a default guava based implementation. Its version shall be the same as of the guava version it uses.
No shading and other hacks are needed. Though another resource (pebble-cache-guava) needs to be published to the maven repository.
It seems that Pebble only constructs Guava's cache in one place and uses it in another. It appears that only the maximum size is used, but its also not clear if other features of the cache are exploited by users.
To the best of my knowledge, no further changes to Guava's cache are planned. However Guava will be evolving rapidly again once it splits into an Android and Java 8 library. Unfortunately the cache under performs because some critical optimizations were planned but never introduced. Those of us who worked on Guava's cache are no longer involved and, now from the outside, I have been completely unsuccessful in getting changes made to correct its flaws.
If Pebble only needs a concurrent, bounded map then you may want to consider ConcurrentLinkedHashMap. That project proved out many of the foundational ideas adopted by Guava, though with higher performance. It is JDK6 compatible, still widely used, small, and has a stable API. However it has reached its EOL due to Caffeine and Guava. It does not support features like expiration, computation, etc. which have to be built on top via decorators.
A custom Cache interface with a default implementation seems reasonable. One might be tempted to adopted JCache but from my experience it is buggy, poorly designed, and a step backwards. I believe it is not GAE compatible because the official API changed since the JSR's original 10+ year old API by the same authors.
Guava has very poor binary compatibility so it is great disadvantage that Pebble use it as leads to unresolvable jar-hell.