> I think we should go the simpler, non-indy route meaning just remove the indy classifier from what we have now and leave the dependencies as-is with version 2.5.2 - yes?
> rather than messing with classifiers and exclusions unless someone can provide some quantitative performance benchmark. given that we have used indy for so long, i almost feel the opposite. like, we should benchmark to ensure a switch provides equivalent performance, no? On Wed, Sep 19, 2018 at 2:30 PM Robert Dale <[email protected]> wrote: > 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 > > >> > > > > > >
