Skip to content

Commit 8daf291

Browse files
mattirnmattirn
committed
Groovy REPL: improve package completion performance
1 parent 8dc496f commit 8daf291

3 files changed

Lines changed: 149 additions & 96 deletions

File tree

‎groovy/src/main/java/org/jline/script/GroovyEngine.java‎

Lines changed: 102 additions & 65 deletions
Original file line numberDiff line numberDiff line change
@@ -221,44 +221,57 @@ public Object execute(File script, Object[] args) throws Exception {
221221
return s.run();
222222
}
223223

224-
private static Set<Class<?>> classesForPackage(String pckgname, GroovyShell shell) throws ClassNotFoundException {
224+
private static Set<Object> classesForPackage(String pckgname, GroovyShell shell) throws ClassNotFoundException {
225225
String name = pckgname;
226226
Matcher matcher = PATTERN_CLASS.matcher(name);
227227
if (matcher.matches()) {
228228
name = matcher.group(1) + ".**";
229229
}
230-
Set<Class<?>> out = new HashSet<>(PackageHelper.getClassesForPackage(name));
230+
Set<Object> out = new HashSet<>(PackageHelper.getClassesForPackage(name));
231231
if (out.isEmpty()) {
232232
out.addAll(JrtJavaBasePackages.getClassesForPackage(name));
233233
}
234234
if (out.isEmpty() && shell != null) {
235-
if (name.endsWith(".*")) {
236-
name = name.substring(0, name.length() - 1);
237-
Set<String> classNames = Helpers.nextFileDomain(name);
235+
EngineClassLoader classLoader = (EngineClassLoader)shell.getClassLoader();
236+
if (pckgname.endsWith(".*")) {
237+
pckgname = pckgname.substring(0, pckgname.length() - 1);
238+
} else if (!pckgname.endsWith(".")) {
239+
pckgname = pckgname + ".";
240+
}
241+
for (Class<?> c : classLoader.getLoadedClasses()) {
242+
String cname = c.getCanonicalName();
243+
if (cname != null && cname.startsWith(pckgname)) {
244+
out.add(c);
245+
}
246+
}
247+
Set<String> classNames = Helpers.sourcesForPackage(name);
248+
if (name.endsWith("*")) {
238249
for (String c : classNames) {
239250
if (Character.isUpperCase(c.charAt(0))) {
240251
try {
241-
out.add((Class<?>) executeStatement(shell, new HashMap<>(), name + c + ".class"));
252+
out.add(executeStatement(shell, new HashMap<>(), pckgname + c + ".class"));
242253
} catch (Exception ignore) {
243254

244255
}
245256
}
246257
}
247-
} else if (name.endsWith(".**")) {
248-
out.addAll(new HashSet<>(PackageHelper.getClassesForPackage(name, shell.getClassLoader()
249-
, n ->
250-
{
251-
if (n.contains("-")) {
252-
return null;
253-
}
254-
Class<?> o = null;
255-
try {
256-
o = (Class<?>) shell.evaluate(n + ".class");
257-
} catch (Exception | Error ignore) {
258-
}
259-
return o;
260-
})));
258+
} else {
259+
out.addAll(classNames);
261260
}
261+
out.addAll(new HashSet<>(PackageHelper.getClassesForPackage(name, shell.getClassLoader()
262+
, n ->
263+
{
264+
if (n.contains("-")) {
265+
return null;
266+
}
267+
Class<?> o = null;
268+
try {
269+
o = (Class<?>) shell.evaluate(n + ".class");
270+
} catch (Exception | Error ignore) {
271+
}
272+
return o;
273+
})));
274+
classLoader.purgeClassCache();
262275
}
263276
return out;
264277
}
@@ -267,10 +280,11 @@ private void addToNameClass(String name) {
267280
addToNameClass(name, nameClass);
268281
}
269282

283+
@SuppressWarnings("unchecked")
270284
private void addToNameClass(String name, Map<String,Class<?>> nameClass) {
271285
try {
272286
if (name.endsWith(".*")) {
273-
for (Class<?> c : classesForPackage(name, shell)) {
287+
for (Class<?> c : (Collection<Class<?>>)(Object)classesForPackage(name, shell)) {
274288
nameClass.put(c.getSimpleName(), c);
275289
}
276290
} else {
@@ -590,6 +604,11 @@ public EngineClassLoader(){
590604
super();
591605
}
592606

607+
@Override
608+
public Package[] getPackages() {
609+
return super.getPackages();
610+
}
611+
593612
public void purgeClassCache(String regex) {
594613
for (String s : classCache.keys()) {
595614
if (s.matches(regex)) {
@@ -627,9 +646,9 @@ public AccessRules(Map<String,Object> options) {
627646

628647
private static class Helpers {
629648

630-
private static Set<String> loadedPackages() {
649+
private static Set<String> loadedPackages(EngineClassLoader classLoader) {
631650
Set<String> out = new HashSet<>();
632-
for (Package p : Package.getPackages()) {
651+
for (Package p : classLoader.getPackages()) {
633652
out.add(p.getName());
634653
}
635654
return out;
@@ -643,7 +662,12 @@ private static Set<String> names(String domain, Collection<String> packages) {
643662
if (idx < 0) {
644663
idx = p.length();
645664
}
646-
out.add(p.substring(domain.length(), idx));
665+
if (idx > domain.length()) {
666+
String name = p.substring(domain.length(), idx);
667+
if (validPackageOrClassName(name)) {
668+
out.add(name);
669+
}
670+
}
647671
}
648672
}
649673
return out;
@@ -717,30 +741,36 @@ private static Map<String,String> getFields(Class<?> clazz, boolean all, boolean
717741
return out;
718742
}
719743

720-
private static Set<String> nextFileDomain(String domain) {
721-
int position = domain != null ? domain.split("\\.").length : 0;
744+
private static Set<String> sourcesForPackage(String domain) {
722745
String separator = FileSystems.getDefault().getSeparator();
723746
if (separator.equals("\\")) {
724747
separator += separator;
725748
}
726749
String dom;
750+
boolean onlyPackage = domain != null && domain.endsWith("*");
751+
if (onlyPackage) {
752+
domain = domain.substring(0, domain.lastIndexOf("."));
753+
}
727754
if (domain != null) {
728-
dom = domain.isEmpty() ? ".*" + separator : separator + domain.replace(".", separator);
755+
dom = domain.isEmpty() ? ".*" + separator : separator + domain.replace(".", separator) + "(|.*)";
729756
} else {
730-
dom = separator;
757+
dom = separator + "(|.*)";
731758
}
732759
PathMatcher matcher = FileSystems.getDefault().getPathMatcher("regex:\\." + dom
733-
+ "[A-Z]+[a-zA-Z]*\\.(groovy|java)");
760+
+ "[A-Z]+[a-zA-Z]*\\.groovy");
734761
Set<String> out = new HashSet<>();
735762
try {
736763
List<Path> paths = Files.walk(Paths.get(".")).filter(matcher::matches).collect(Collectors.toList());
737764
for (Path p : paths) {
738-
if (!p.getFileName().toString().matches("[A-Z]+[a-zA-Z]*\\.(groovy|java)")) {
765+
if (!p.getFileName().toString().matches("[A-Z]+[a-zA-Z]*\\.groovy")) {
739766
continue;
740767
}
741-
String[] s = p.toString().split(separator);
742-
if (s.length > position + 1) {
743-
out.add(s[position + 1].split(".(groovy|java)")[0]);
768+
String source = p.toString();
769+
String className = source.substring(2, source.lastIndexOf(".")).replace(separator, ".");
770+
if (onlyPackage && Character.isUpperCase(className.charAt(domain.length() + 1))) {
771+
out.add(className);
772+
} else {
773+
out.add(className);
744774
}
745775
}
746776
} catch(Exception ignore) {
@@ -755,60 +785,69 @@ public static Set<String> nextDomain(String domain, CandidateType type, GroovySh
755785

756786
public static Set<String> nextDomain(String domain, AccessRules access, CandidateType type, GroovyShell shell) {
757787
Set<String> out = new HashSet<>();
788+
EngineClassLoader classLoader = (EngineClassLoader)shell.getClassLoader();
758789
if (domain.isEmpty()) {
759-
for (String p : loadedPackages()) {
790+
for (String p : loadedPackages(classLoader)) {
760791
out.add(p.split("\\.")[0]);
761792
}
762-
out.addAll(nextFileDomain(null));
763-
} else if ((domain.split("\\.")).length < 2) {
764-
out = names(domain, loadedPackages());
765-
out.addAll(names(domain, PackageHelper.getClassNamesForPackage(domain, shell.getClassLoader())));
766-
out.addAll(nextFileDomain(domain));
793+
out.addAll(names(domain, sourcesForPackage(null)));
767794
} else {
768795
try {
769-
if (!domain.matches(REGEX_CLASS)) {
770-
out = names(domain, PackageHelper.getClassNamesForPackage(domain, shell.getClassLoader()));
771-
}
772-
for (Class<?> c : classesForPackage(domain, shell)) {
773-
try {
774-
if ((!Modifier.isPublic(c.getModifiers()) && !access.allClasses) || c.getCanonicalName() == null) {
775-
continue;
776-
}
777-
if ((type == CandidateType.CONSTRUCTOR && (c.getConstructors().length == 0
778-
|| Modifier.isAbstract(c.getModifiers())))
779-
|| (type == CandidateType.STATIC_METHOD && noStaticMethods(c, access.allMethods)
780-
&& noStaticFields(c, access.allFields))) {
781-
continue;
796+
for (Object o : classesForPackage(domain, shell)) {
797+
String name = null;
798+
if (o instanceof Class<?>) {
799+
Class<?> c = (Class<?>) o;
800+
try {
801+
if ((!Modifier.isPublic(c.getModifiers()) && !access.allClasses) || c.getCanonicalName() == null) {
802+
continue;
803+
}
804+
if ((type == CandidateType.CONSTRUCTOR && (c.getConstructors().length == 0
805+
|| Modifier.isAbstract(c.getModifiers())))
806+
|| (type == CandidateType.STATIC_METHOD && noStaticMethods(c, access.allMethods)
807+
&& noStaticFields(c, access.allFields))) {
808+
continue;
809+
}
810+
name = c.getCanonicalName();
811+
} catch (NoClassDefFoundError e) {
812+
if (Log.isDebugEnabled()) {
813+
e.printStackTrace();
814+
}
782815
}
783-
String name = c.getCanonicalName();
816+
} else if (o instanceof String) {
817+
name = ((String)o).replace("$", ".");
818+
}
819+
if (name != null) {
784820
Log.debug(name);
785821
if (name.startsWith(domain)) {
786822
int idx = name.indexOf('.', domain.length());
787823
if (idx < 0) {
788824
idx = name.length();
789825
}
790-
out.add(name.substring(domain.length(), idx));
791-
}
792-
} catch (NoClassDefFoundError e) {
793-
if (Log.isDebugEnabled()) {
794-
e.printStackTrace();
826+
if (idx > domain.length()) {
827+
name = name.substring(domain.length(), idx);
828+
if (validPackageOrClassName(name)) {
829+
out.add(name);
830+
}
831+
}
795832
}
796833
}
797834
}
798-
if (out.isEmpty() && type != CandidateType.PACKAGE) {
799-
out.addAll(nextFileDomain(domain));
800-
}
801835
} catch (ClassNotFoundException e) {
802836
if (Log.isDebugEnabled()) {
803837
e.printStackTrace();
804838
}
805-
out.addAll(names(domain, loadedPackages()));
806-
out.addAll(names(domain, PackageHelper.getClassNamesForPackage(domain, shell.getClassLoader())));
839+
out.addAll(names(domain, loadedPackages(classLoader)));
840+
out.addAll(names(domain, PackageHelper.getClassNamesForPackage(domain, classLoader)));
807841
}
808842
}
843+
Log.debug("nextDomain: ", out);
809844
return out;
810845
}
811846

847+
private static boolean validPackageOrClassName(String name) {
848+
return !(name.contains("-") || name.matches("\\d+.*"));
849+
}
850+
812851
private static Map<String, String> listToMap(Collection<String> list) {
813852
return list.stream()
814853
.collect(Collectors.toMap(it -> it, it -> ""));
@@ -1213,7 +1252,6 @@ private Set<String> retrieveConstructors(boolean all) {
12131252
it.remove();
12141253
}
12151254
}
1216-
out.addAll(Helpers.nextFileDomain(null));
12171255
return out;
12181256
}
12191257

@@ -1231,7 +1269,6 @@ private Set<String> retrieveClassesWithStaticMethods() {
12311269
it.remove();
12321270
}
12331271
}
1234-
out.addAll(Helpers.nextFileDomain(null));
12351272
return out;
12361273
}
12371274
}

‎groovy/src/main/java/org/jline/script/JrtJavaBasePackages.java‎

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2002-2020, the original author or authors.
2+
* Copyright (c) 2002-2021, the original author or authors.
33
*
44
* This software is distributable under the BSD license. See the terms of the
55
* BSD license in the documentation provided with this software.
@@ -26,7 +26,7 @@
2626
* @author <a href="mailto:matti.rintanikkola@gmail.com">Matti Rinta-Nikkola</a>
2727
*/
2828
public class JrtJavaBasePackages {
29-
public static List<Class<?>> getClassesForPackage(String pckgname) {
29+
public static List<Object> getClassesForPackage(String pckgname) {
3030
FileSystem fs = FileSystems.getFileSystem(URI.create("jrt:/"));
3131
List<String> dirs = new ArrayList<>();
3232
dirs.add("java.base");
@@ -42,7 +42,7 @@ public static List<Class<?>> getClassesForPackage(String pckgname) {
4242
}
4343
dirs.addAll(Arrays.asList(pckgname.split("\\.")));
4444
Path path = fs.getPath("modules", dirs.toArray(new String[0]));
45-
FileVisitor fv = new FileVisitor(nestedClasses);
45+
FileVisitor fv = new FileVisitor(pckgname, nestedClasses);
4646
try {
4747
if (onlyCurrent) {
4848
Files.walkFileTree(path, new HashSet<>(),1, fv);
@@ -58,11 +58,13 @@ public static List<Class<?>> getClassesForPackage(String pckgname) {
5858
}
5959

6060
private static class FileVisitor extends SimpleFileVisitor<Path> {
61-
private final List<Class<?>> classes = new ArrayList<>();
61+
private final List<Object> classes = new ArrayList<>();
6262
private final boolean nestedClasses;
63+
private final String pckgname;
6364

64-
public FileVisitor(boolean nestedClasses) {
65+
public FileVisitor(String pckgname, boolean nestedClasses) {
6566
super();
67+
this.pckgname = pckgname;
6668
this.nestedClasses = nestedClasses;
6769
}
6870

@@ -71,7 +73,12 @@ public FileVisitResult visitFile(Path file, BasicFileAttributes attr) {
7173
try {
7274
String name = file.toString().substring(18);
7375
if (name.endsWith(".class") && (nestedClasses || !name.contains("$"))) {
74-
classes.add(Class.forName(name.substring(0, name.length() - 6).replaceAll("/", ".")));
76+
String className = name.substring(0, name.length() - 6).replaceAll("/", ".");
77+
if (Character.isUpperCase(className.charAt(pckgname.length() + 1))) {
78+
classes.add(Class.forName(className));
79+
} else {
80+
classes.add(className);
81+
}
7582
}
7683
} catch (Exception|Error e) {
7784
if (Log.isDebugEnabled()) {
@@ -81,7 +88,7 @@ public FileVisitResult visitFile(Path file, BasicFileAttributes attr) {
8188
return CONTINUE;
8289
}
8390

84-
private List<Class<?>> getClasses() {
91+
private List<Object> getClasses() {
8592
return classes;
8693
}
8794
}

0 commit comments

Comments
 (0)