As far as I can tell, 2.5 is in the same boat as 2.4 where we would have to specifically include all 'indy' dependencies while excluding the non-indy deps of those deps. And because there is no groovy-all jar any more, it's no longer as easy as including -all indy.
I couldn't find much on using 'indy' with maven. That is, no hints, questions, or complaints about conflicts with including indy and non-indy jars in the same project. I looked at some popular projects - spring, logback, etc - and none use indy. And maybe that's for java version compatibility more than anything else. I don't know. So those that are using indy, like us, may not be aware that both jars are included and is unhealthy. Why do we need invokedymanic anyway? Gremlin groovy compiler does not set the invokedymanic flag, so gremlin scripts are not benefiting from it directly there. The Gremlin Console also does not enable indy. To be honest, I'm not even aware of the actual use cases of where invokedymanic kicks in. So it's hard for me to say what, if any, benefit it would provide with Gremlin scripts. Within groovy itself, indy only applies to those parts of groovy which are actually written in groovy and not java. There is only a small percentage of groovy written in groovy. Of the dependencies we use, 'src/main/' of each module: groovy: java: 797 groovy: 43 subprojects/groovy-ant java: 15 groovy: 1 subprojects/groovy-json java: 45 groovy: 0 subprojects/groovy-jsr223 java: 5 groovy: 0 subprojects/groovy-groovysh java: 0 groovy: 61 One module that's written entirely in groovy is groovysh. AFAIK, the only place it's used is in Gremlin Console which does not enable indy (neither does groovy console by default). It would seem there is little performance benefit here because how much work does the console actually do? We're talking about the core functions of the console separate from running scripts and compiling scripts (which obviously comes from the compiler which, again, does not enable indy by default). I think we should go the simpler, non-indy route rather than messing with classifiers and exclusions unless someone can provide some quantitative performance benchmark. If there are complaints after the fact - dude, 3.4.0 sucks cuz my scripts run so slow - then we can always back it out and go the indy route. Robert Dale On Wed, Sep 19, 2018 at 10:55 AM Stephen Mallette <[email protected]> wrote: > no idea what "UNCHECKED" in the subject header means........something > apache mailing list put there................... > > On Wed, Sep 19, 2018 at 10:49 AM Stephen Mallette <[email protected]> > wrote: > > > Created this: > > > > https://issues.apache.org/jira/browse/TINKERPOP-2039 > > > > On Tue, Sep 18, 2018 at 7:48 PM Robert Dale <[email protected]> wrote: > > > >> Sorry, was skimming and only saw the 3.0 mention. The default is in > 3.0. > >> I > >> thought I read that was new there. I don’t know what’s in 2.5. I’ll > try > >> to take a look this week. > >> > >> On Tue, Sep 18, 2018 at 19:20 Stephen Mallette <[email protected]> > >> wrote: > >> > >> > ah - i tried to find some specific mention of that and didn't come > >> across > >> > it. if that's the case, then i think that maybe we should just head > >> > straight to 2.5.2 on master. would you still go to groovy-all if we > did > >> > that? i presume the indy problem would go away in that case then > right? > >> > > >> > i do think we should just do this all at once as part of one PR - i > have > >> > 2.5.2. building now with some minor required changes to get it > working. > >> i > >> > can push that to a branch and we can go from there.... > >> > > >> > On Tue, Sep 18, 2018 at 7:16 PM Robert Dale <[email protected]> > wrote: > >> > > >> > > Indy is gone because invokedynamic is the default in that version. > >> > > > >> > > On Tue, Sep 18, 2018 at 18:30 Stephen Mallette < > [email protected]> > >> > > wrote: > >> > > > >> > > > Looking ahead a bit on this one (I'm interested in what it will > >> take to > >> > > get > >> > > > to Groovy 3.0.0) I noticed that 2.5.x requires <type>pom</type> > and > >> > > > that groovy-all-x.y.z.jar is no longer available "In order to > cater > >> to > >> > > the > >> > > > module system of Java 9+" - i also don't see an "indy" classifier > >> for > >> > > that > >> > > > in central: > >> > > > > >> > > > > >> https://repo1.maven.org/maven2/org/codehaus/groovy/groovy-all/2.5.2/ > >> > > > > >> > > > atm, i'm not sure if that changes anything in terms of the > >> direction we > >> > > > were headed. > >> > > > > >> > > > On Mon, Sep 17, 2018 at 3:06 PM Stephen Mallette < > >> [email protected] > >> > > > >> > > > wrote: > >> > > > > >> > > > > jeez - hornets nest > >> > > > > > >> > > > > interesting that going to a single module makes our distribution > >> > > smaller. > >> > > > > I assume that we must have though the opposite when we started > >> using > >> > > > pieces > >> > > > > of groovy rather than "-all". at this point there shouldn't be > >> any > >> > > more > >> > > > > weirdly licensed files in groovy given that they have been > >> releasing > >> > > > under > >> > > > > apache for as long as we have. i can't really come up with a > >> reason > >> > not > >> > > > to > >> > > > > go to "-all" on 3.4.0, so i guess i'm +1 for that. > >> > > > > > >> > > > > the :install command could support classifiers because grape/ivy > >> > > support > >> > > > > it as an option. we could treat that as a separate issue i > guess - > >> > new > >> > > > JIRA? > >> > > > > > >> > > > > > >> > > > > > >> > > > > > >> > > > > > >> > > > > On Sun, Sep 16, 2018 at 6:00 PM Robert Dale <[email protected]> > >> > wrote: > >> > > > > > >> > > > >> So, I was taking a closer look at this there's potentially a > >> larger > >> > > mess > >> > > > >> we > >> > > > >> want to address. > >> > > > >> > >> > > > >> TLDR: instead of removing groovy-sql, replace all groovy deps > >> with > >> > > > >> groovy-all-indy. > >> > > > >> > >> > > > >> First, a bit of history. tp32 did actually use groovy.sql.Sql > >> > > > >> in ConsoleImportCustomizerProvider. This class was deprecated > in > >> > > 3.2.4. > >> > > > >> Later, it was removed in 3.3.0. > >> > > > >> > >> > > > >> With the plan being to remove groovy-sql completely in 3.4.0, I > >> was > >> > > > >> testing > >> > > > >> my instructions on using the :install feature to reinstall it > >> just > >> > in > >> > > > case > >> > > > >> had someone depended on it. It of course downloaded groovy-sql > >> but > >> > it > >> > > > was > >> > > > >> different than what was in lib/. It was missing '-indy'. It > >> also > >> > > > >> downloaded the non-indy version of groovy. I had to go learn > >> what > >> > > > '-indy' > >> > > > >> was about. For those who don't know '-indy' is a maven > >> classifier to > >> > > > >> denote > >> > > > >> that the jar was built with Java 1.7 'invokedymanic' support as > >> > > opposed > >> > > > >> the > >> > > > >> legacy 'call site'. The short of it is that invokedymanic is > >> more > >> > > > >> performant than call site. > >> > > > >> > >> > > > >> As I was comparing groovy lib/, I noticed that console and > server > >> > > > already > >> > > > >> include both groovy-2.4.15-indy.jar and groovy-2.4.15.jar. > >> However, > >> > > the > >> > > > >> docs[1] state that only one or the other should be included. > Same > >> > goes > >> > > > for > >> > > > >> any 'indy' and 'non-indy' jar of the same module. Luckily, we > >> don't > >> > > > have > >> > > > >> any other conflict like that although there is a mix of other > >> indy > >> > > > >> non-indy > >> > > > >> modules. > >> > > > >> > >> > > > >> I tried again and was unable to download the indy version of > >> > > groovy-sql > >> > > > >> because the :install command does not allow a 'classifier' > >> > parameter. > >> > > It > >> > > > >> will only download the non-indy verison of modules. Even if it > >> > could > >> > > > >> download the 'indy' version, the groovy dependencies are built > >> such > >> > > that > >> > > > >> all 'indy' modules have a dependency on non-indy modules. > >> > > > >> > >> > > > >> So, there are several issues: > >> > > > >> 1) there is included a conflicting mix of indy and non-indy > >> builds > >> > of > >> > > > the > >> > > > >> same module - groovy. This is bad and should be fixed. > >> > > > >> 2) there is included a mix of indy and non-indy builds of > >> different > >> > > > >> modules. This is not necessarily bad but does mean we're using > >> > > > potentially > >> > > > >> less performant pieces of groovy. > >> > > > >> indy) groovysh, json, jsr223 > >> > > > >> non-indy) console, swing, templates, xml > >> > > > >> 3) Install command doesn't support classifier parameter. I'm > not > >> > > aware > >> > > > of > >> > > > >> anyone complaining about this before so it's not necessarily an > >> > issue > >> > > in > >> > > > >> itself. However, if we were to desire that all groovy modules > be > >> > > > >> indy-only, > >> > > > >> then classifier support would be required. But then I don't > know > >> > how > >> > > to > >> > > > >> prevent the non-indy dependencies of that module from being > >> > > downloaded. > >> > > > It > >> > > > >> seems like a mix would always occur. > >> > > > >> > >> > > > >> There is one simple solution interestingly enough: depend only > >> on > >> > > > >> groovy-all-indy. Not only does this keep all indy modules > >> > consistent > >> > > > and > >> > > > >> prevent non-indy conflicts/duplicates, it actually shrinks the > >> > > > >> distribution > >> > > > >> size by 4M. This is from not distributing the duplicate > non-indy > >> > > > version > >> > > > >> of groovy and because groovy-all isn't much bigger than the > >> separate > >> > > > >> modules we have now. The other benefit is that we can reduce > >> groovy > >> > > > >> dependencies down to a single module across the project. The > >> > > > alternative > >> > > > >> is ugly - having a direct dependency on each module in the > >> > dependency > >> > > > tree > >> > > > >> and having an excludes on each of those dependencies to prevent > >> them > >> > > > from > >> > > > >> loading the non-indy deps. > >> > > > >> > >> > > > >> In conclusion, instead of removing groovy-sql, I am proposing > >> that > >> > > > instead > >> > > > >> we replace all groovy deps with groovy-all-indy. > >> > > > >> > >> > > > >> Thoughts? > >> > > > >> > >> > > > >> Stephen, are the groovy scripts submitted to Gremlin Server > >> compiled > >> > > > with > >> > > > >> invokedynamic? What about Gremlin Console? I know the groovy > >> > console > >> > > > does > >> > > > >> not have this enabled by default. > >> > > > >> > >> > > > >> 1. > >> > > > >> > >> > > > >> > >> > > > > >> > > > >> > > >> > http://docs.groovy-lang.org/docs/groovy-2.4.15/html/documentation/invokedynamic-support.html#_two_jars > >> > > > >> > >> > > > >> Robert Dale > >> > > > >> > >> > > > >> > >> > > > >> On Fri, Sep 14, 2018 at 9:32 AM Robert Dale <[email protected] > > > >> > > wrote: > >> > > > >> > >> > > > >> > No, nothing in the docs. There are some references to > >> > > > java.sql.Timestamp > >> > > > >> > but only within the context of gryo. I'll just make an > upgrade > >> > note > >> > > > >> that > >> > > > >> > it was removed and if required, it can be installed through > the > >> > > > :install > >> > > > >> > command. > >> > > > >> > > >> > > > >> > Robert Dale > >> > > > >> > > >> > > > >> > > >> > > > >> > On Fri, Sep 14, 2018 at 6:34 AM Stephen Mallette < > >> > > > [email protected]> > >> > > > >> > wrote: > >> > > > >> > > >> > > > >> >> I think groovy-sql was there because there was a time when I > >> was > >> > > into > >> > > > >> >> polyglot data transforms in Gremlin and groovy-sql enabled > >> you to > >> > > > >> connect > >> > > > >> >> to JDBC data sources in a nice way. So, I included > groovy-sql > >> as > >> > a > >> > > > >> >> convenience. I'm not against removing it, though I think we > >> > should > >> > > > >> remove > >> > > > >> >> it in 3.4.0 only in case someone is currently depending on > it > >> > > > >> indirectly. > >> > > > >> >> Please try to double check that we aren't using groovy-sql > in > >> any > >> > > of > >> > > > >> the > >> > > > >> >> documentation for some odd example or something. > >> > > > >> >> > >> > > > >> >> On Thu, Sep 13, 2018 at 5:20 PM Robert Dale < > >> [email protected]> > >> > > > wrote: > >> > > > >> >> > >> > > > >> >> > gremlin-driver has a direct dependency on groovy-sql. I > >> > removed > >> > > it > >> > > > >> in > >> > > > >> >> > master and all tests pass (docker/build.sh -i -t -n) so > >> there > >> > > does > >> > > > >> not > >> > > > >> >> > appear to be any direct or indirect usage. > >> > > > >> >> > > >> > > > >> >> > Why is it there? Looks like it was pulled in because of > >> > > > >> >> > https://issues.apache.org/jira/browse/TINKERPOP-713 > >> > > > >> >> > However, groovy-sql does not appear to be a dependency of > >> > > > >> >> groovy-console or > >> > > > >> >> > anything groovy (except groovy-all). Wasn't then, isn't > now. > >> > > > >> >> > > >> > > > >> >> > Any objections to removing it? > >> > > > >> >> > > >> > > > >> >> > -- > >> > > > >> >> > Robert Dale > >> > > > >> >> > > >> > > > >> >> > >> > > > >> > > >> > > > >> > >> > > > > > >> > > > > >> > > -- > >> > > Robert Dale > >> > > > >> > > >> -- > >> Robert Dale > >> > > >
