You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@geronimo.apache.org by "struberg (via GitHub)" <gi...@apache.org> on 2026/07/06 15:40:46 UTC
[PR] Fb perf [geronimo-xbean]
struberg opened a new pull request, #57:
URL: https://github.com/apache/geronimo-xbean/pull/57
a few performance patches.
I've tested them with OWB and TomEE. Please take a look before I push it to main.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@geronimo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fb perf [geronimo-xbean]
Posted by "rzo1 (via GitHub)" <gi...@apache.org>.
rzo1 commented on code in PR #57:
URL: https://github.com/apache/geronimo-xbean/pull/57#discussion_r3531211437
##########
xbean-finder/src/main/java/org/apache/xbean/finder/ClassFinder.java:
##########
@@ -200,28 +195,20 @@ private List<String> jar(URL location) throws IOException {
InputStream in = url.openStream();
try {
JarInputStream jarStream = new JarInputStream(in);
- return jar(jarStream);
+ jar(jarStream);
} finally {
in.close();
}
}
- private List<String> jar(JarInputStream jarStream) throws IOException {
- List<String> classNames = new ArrayList<String>();
-
+ private void jar(JarInputStream jarStream) throws IOException {
JarEntry entry;
while ((entry = jarStream.getNextJarEntry()) != null) {
if (entry.isDirectory() || !entry.getName().endsWith(".class")) {
continue;
}
- String className = entry.getName();
- className = className.replaceFirst(".class$", "");
- if (className.contains(".")) continue;
- className = className.replace('/', '.');
- classNames.add(className);
+ readClassDef(jarStream);
Review Comment:
Think its narrower than at the first glance as discussed via Slack ;-) - just ignore that.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@geronimo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fb perf [geronimo-xbean]
Posted by "rzo1 (via GitHub)" <gi...@apache.org>.
rzo1 commented on code in PR #57:
URL: https://github.com/apache/geronimo-xbean/pull/57#discussion_r3530853791
##########
xbean-finder/src/main/java/org/apache/xbean/finder/ClassFinder.java:
##########
@@ -162,36 +157,36 @@ protected Class<?> loadClass(String fixedName) throws ClassNotFoundException {
- private List<String> file(URL location) {
- List<String> classNames = new ArrayList<String>();
+ private void file(URL location) throws IOException {
File dir = new File(URLDecoder.decode(location.getPath()));
if (dir.getName().equals("META-INF")) {
dir = dir.getParentFile(); // Scrape "META-INF" off
}
if (dir.isDirectory()) {
- scanDir(dir, classNames, "");
+ scanDir(dir);
}
- return classNames;
}
- private void scanDir(File dir, List<String> classNames, String packageName) {
+ private void scanDir(File dir) throws IOException {
File[] files = dir.listFiles();
if (files == null) {
return;
}
for (File file : files) {
if (file.isDirectory()) {
- scanDir(file, classNames, packageName + file.getName() + ".");
+ scanDir(file);
} else if (file.getName().endsWith(".class")) {
- String name = file.getName();
- name = name.replaceFirst(".class$", "");
- if (name.contains(".")) continue;
Review Comment:
The removed if (name.contains(".")) continue; filters mean non-classfile artifacts named *.class are now fed to ASM. E.g. a macOS AppleDouble sidecar ._Foo.class was previously skipped before any bytes were read; now ClassReader throws a RuntimeException on the garbage bytes
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@geronimo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fb perf [geronimo-xbean]
Posted by "rzo1 (via GitHub)" <gi...@apache.org>.
rzo1 commented on code in PR #57:
URL: https://github.com/apache/geronimo-xbean/pull/57#discussion_r3530909103
##########
xbean-finder/src/main/java/org/apache/xbean/finder/ClassFinder.java:
##########
@@ -200,28 +195,20 @@ private List<String> jar(URL location) throws IOException {
InputStream in = url.openStream();
try {
JarInputStream jarStream = new JarInputStream(in);
- return jar(jarStream);
+ jar(jarStream);
} finally {
in.close();
}
}
- private List<String> jar(JarInputStream jarStream) throws IOException {
- List<String> classNames = new ArrayList<String>();
-
+ private void jar(JarInputStream jarStream) throws IOException {
JarEntry entry;
while ((entry = jarStream.getNextJarEntry()) != null) {
if (entry.isDirectory() || !entry.getName().endsWith(".class")) {
continue;
}
- String className = entry.getName();
- className = className.replaceFirst(".class$", "");
- if (className.contains(".")) continue;
- className = className.replace('/', '.');
- classNames.add(className);
+ readClassDef(jarStream);
Review Comment:
Previously class names were collected first and each one was read via readClassDef(String), which caught IOException per class and just recorded it in classesNotLoaded. Now readClassDef(InputStream) is called inline from the scan loops, so a single bad .class entry (corrupt/truncated entry, unreadable file) propagates out of jar(JarInputStream) / scanDir(File) and is only caught by the per-URL catch (Exception e) { e.printStackTrace(); } in the constructor - silently skipping all remaining entries of that jar/directory.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@geronimo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fb perf [geronimo-xbean]
Posted by "struberg (via GitHub)" <gi...@apache.org>.
struberg commented on code in PR #57:
URL: https://github.com/apache/geronimo-xbean/pull/57#discussion_r3531169206
##########
xbean-finder/src/main/java/org/apache/xbean/finder/ClassFinder.java:
##########
@@ -200,28 +195,20 @@ private List<String> jar(URL location) throws IOException {
InputStream in = url.openStream();
try {
JarInputStream jarStream = new JarInputStream(in);
- return jar(jarStream);
+ jar(jarStream);
} finally {
in.close();
}
}
- private List<String> jar(JarInputStream jarStream) throws IOException {
- List<String> classNames = new ArrayList<String>();
-
+ private void jar(JarInputStream jarStream) throws IOException {
JarEntry entry;
while ((entry = jarStream.getNextJarEntry()) != null) {
if (entry.isDirectory() || !entry.getName().endsWith(".class")) {
continue;
}
- String className = entry.getName();
- className = className.replaceFirst(".class$", "");
- if (className.contains(".")) continue;
- className = className.replace('/', '.');
- classNames.add(className);
+ readClassDef(jarStream);
Review Comment:
u sure?
Previous code was:
```
for (String className : classNames) {
readClassDef(className);
}
```
``readClassDef`` catches Exceptions internally already, so not sure it applies? Do I overlook something?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@geronimo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fb perf [geronimo-xbean]
Posted by "struberg (via GitHub)" <gi...@apache.org>.
struberg commented on code in PR #57:
URL: https://github.com/apache/geronimo-xbean/pull/57#discussion_r3531125006
##########
xbean-finder/src/main/java/org/apache/xbean/finder/ClassFinder.java:
##########
@@ -162,36 +157,36 @@ protected Class<?> loadClass(String fixedName) throws ClassNotFoundException {
- private List<String> file(URL location) {
- List<String> classNames = new ArrayList<String>();
+ private void file(URL location) throws IOException {
File dir = new File(URLDecoder.decode(location.getPath()));
if (dir.getName().equals("META-INF")) {
dir = dir.getParentFile(); // Scrape "META-INF" off
}
if (dir.isDirectory()) {
- scanDir(dir, classNames, "");
+ scanDir(dir);
}
- return classNames;
}
- private void scanDir(File dir, List<String> classNames, String packageName) {
+ private void scanDir(File dir) throws IOException {
File[] files = dir.listFiles();
if (files == null) {
return;
}
for (File file : files) {
if (file.isDirectory()) {
- scanDir(file, classNames, packageName + file.getName() + ".");
+ scanDir(file);
} else if (file.getName().endsWith(".class")) {
- String name = file.getName();
- name = name.replaceFirst(".class$", "");
- if (name.contains(".")) continue;
Review Comment:
now solved with a
```
if (name.indexOf('.') != name.length()-6) {
continue;
}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@geronimo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fb perf [geronimo-xbean]
Posted by "struberg (via GitHub)" <gi...@apache.org>.
struberg commented on code in PR #57:
URL: https://github.com/apache/geronimo-xbean/pull/57#discussion_r3531346352
##########
xbean-finder/src/main/java/org/apache/xbean/finder/ClassFinder.java:
##########
@@ -200,28 +195,20 @@ private List<String> jar(URL location) throws IOException {
InputStream in = url.openStream();
try {
JarInputStream jarStream = new JarInputStream(in);
- return jar(jarStream);
+ jar(jarStream);
} finally {
in.close();
}
}
- private List<String> jar(JarInputStream jarStream) throws IOException {
- List<String> classNames = new ArrayList<String>();
-
+ private void jar(JarInputStream jarStream) throws IOException {
JarEntry entry;
while ((entry = jarStream.getNextJarEntry()) != null) {
if (entry.isDirectory() || !entry.getName().endsWith(".class")) {
continue;
}
- String className = entry.getName();
- className = className.replaceFirst(".class$", "");
- if (className.contains(".")) continue;
- className = className.replace('/', '.');
- classNames.add(className);
+ readClassDef(jarStream);
Review Comment:
Thanks for pointing it out anyway, I actually didn't have subtle behaviour changes regarding the Exceptions on the radar - txs!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@geronimo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fb perf [geronimo-xbean]
Posted by "asf-gitbox-commits (via GitHub)" <gi...@apache.org>.
asf-gitbox-commits merged PR #57:
URL: https://github.com/apache/geronimo-xbean/pull/57
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@geronimo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fb perf [geronimo-xbean]
Posted by "struberg (via GitHub)" <gi...@apache.org>.
struberg commented on code in PR #57:
URL: https://github.com/apache/geronimo-xbean/pull/57#discussion_r3531125006
##########
xbean-finder/src/main/java/org/apache/xbean/finder/ClassFinder.java:
##########
@@ -162,36 +157,36 @@ protected Class<?> loadClass(String fixedName) throws ClassNotFoundException {
- private List<String> file(URL location) {
- List<String> classNames = new ArrayList<String>();
+ private void file(URL location) throws IOException {
File dir = new File(URLDecoder.decode(location.getPath()));
if (dir.getName().equals("META-INF")) {
dir = dir.getParentFile(); // Scrape "META-INF" off
}
if (dir.isDirectory()) {
- scanDir(dir, classNames, "");
+ scanDir(dir);
}
- return classNames;
}
- private void scanDir(File dir, List<String> classNames, String packageName) {
+ private void scanDir(File dir) throws IOException {
File[] files = dir.listFiles();
if (files == null) {
return;
}
for (File file : files) {
if (file.isDirectory()) {
- scanDir(file, classNames, packageName + file.getName() + ".");
+ scanDir(file);
} else if (file.getName().endsWith(".class")) {
- String name = file.getName();
- name = name.replaceFirst(".class$", "");
- if (name.contains(".")) continue;
Review Comment:
now solved with a
```
if (name.indexOf('.') != name.length()-6) {
continue;
}
```
This has the benefit of not performing a potential memory allocation but only reading in the mem.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@geronimo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fb perf [geronimo-xbean]
Posted by "rzo1 (via GitHub)" <gi...@apache.org>.
rzo1 commented on code in PR #57:
URL: https://github.com/apache/geronimo-xbean/pull/57#discussion_r3530909103
##########
xbean-finder/src/main/java/org/apache/xbean/finder/ClassFinder.java:
##########
@@ -200,28 +195,20 @@ private List<String> jar(URL location) throws IOException {
InputStream in = url.openStream();
try {
JarInputStream jarStream = new JarInputStream(in);
- return jar(jarStream);
+ jar(jarStream);
} finally {
in.close();
}
}
- private List<String> jar(JarInputStream jarStream) throws IOException {
- List<String> classNames = new ArrayList<String>();
-
+ private void jar(JarInputStream jarStream) throws IOException {
JarEntry entry;
while ((entry = jarStream.getNextJarEntry()) != null) {
if (entry.isDirectory() || !entry.getName().endsWith(".class")) {
continue;
}
- String className = entry.getName();
- className = className.replaceFirst(".class$", "");
- if (className.contains(".")) continue;
- className = className.replace('/', '.');
- classNames.add(className);
+ readClassDef(jarStream);
Review Comment:
Previously class names were collected first and each one was read via readClassDef(String), which caught IOException per class and just recorded it in classesNotLoaded. Now readClassDef(InputStream) is called inline from the scan loops, so a single bad .class entry (corrupt/truncated entry, unreadable file) propagates out of jar(JarInputStream) / scanDir(File) and is only caught by the per-URL catch (Exception e) { e.printStackTrace(); } in the constructor - silently skipping all remaining entries of that jar/directory.
Perhaps just try/catch it to have previous behaviour?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@geronimo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fb perf [geronimo-xbean]
Posted by "rmannibucau (via GitHub)" <gi...@apache.org>.
rmannibucau commented on code in PR #57:
URL: https://github.com/apache/geronimo-xbean/pull/57#discussion_r3530890935
##########
xbean-finder/src/main/java/org/apache/xbean/finder/ClassFinder.java:
##########
@@ -162,36 +157,36 @@ protected Class<?> loadClass(String fixedName) throws ClassNotFoundException {
- private List<String> file(URL location) {
- List<String> classNames = new ArrayList<String>();
+ private void file(URL location) throws IOException {
File dir = new File(URLDecoder.decode(location.getPath()));
if (dir.getName().equals("META-INF")) {
dir = dir.getParentFile(); // Scrape "META-INF" off
}
if (dir.isDirectory()) {
- scanDir(dir, classNames, "");
+ scanDir(dir);
}
- return classNames;
}
- private void scanDir(File dir, List<String> classNames, String packageName) {
+ private void scanDir(File dir) throws IOException {
File[] files = dir.listFiles();
if (files == null) {
return;
}
for (File file : files) {
if (file.isDirectory()) {
- scanDir(file, classNames, packageName + file.getName() + ".");
+ scanDir(file);
} else if (file.getName().endsWith(".class")) {
- String name = file.getName();
- name = name.replaceFirst(".class$", "");
- if (name.contains(".")) continue;
Review Comment:
we should just !startsWith(".") too
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@geronimo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org