Skip to content

Commit

Permalink
Optimized performance in getSourcePositionFromStack
Browse files Browse the repository at this point in the history
  • Loading branch information
@rPraml @rbri
rPraml authored and rbri committed Oct 4, 2024
1 parent b5d53fd commit f3f1b3f
Showing 1 changed file with 23 additions and 16 deletions.
39 changes: 23 additions & 16 deletions rhino/src/main/java/org/mozilla/javascript/Context.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
import java.util.HashSet;
import java.util.Locale;
import java.util.Map;
import java.util.Optional;
import java.util.Set;
import java.util.TimeZone;
import java.util.function.UnaryOperator;
Expand Down Expand Up @@ -2568,23 +2569,29 @@ static String getSourcePositionFromStack(int[] linep) {
Evaluator evaluator = createInterpreter();
if (evaluator != null) return evaluator.getSourcePositionFromStack(cx, linep);
}
/**
* A bit of a hack, but the only way to get filename and line number from an enclosing
* frame.
*/
StackTraceElement[] stackTrace = new Throwable().getStackTrace();
for (StackTraceElement st : stackTrace) {
String file = st.getFileName();
if (!(file == null || file.endsWith(".java"))) {
int line = st.getLineNumber();
if (line >= 0) {
linep[0] = line;
return file;
}
}
}

return null;
return getSourcePositionFromJavaStack(linep);
}

/** Returns the current filename in the java stack. */
@SuppressWarnings("AndroidJdkLibsChecker")
// Android uses interpreter, so we should not get here.
static String getSourcePositionFromJavaStack(int[] linep) {
Optional<StackWalker.StackFrame> frame =
StackWalker.getInstance()
.walk(stream -> stream.filter(Context::frameMatches).findFirst());
return frame.map(
f -> {
linep[0] = f.getLineNumber();
return f.getFileName();
})
.orElse(null);
}

@SuppressWarnings("AndroidJdkLibsChecker")
private static boolean frameMatches(StackWalker.StackFrame frame) {
return (frame.getFileName() == null || !frame.getFileName().endsWith(".java"))
&& frame.getLineNumber() > 0;
}

RegExpProxy getRegExpProxy() {
Expand Down

4 comments on commit f3f1b3f

@anonyein
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This commit cannot perform well on Android system!

@rbri
Copy link
Collaborator

@rbri rbri commented on f3f1b3f Oct 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@anonyein can you please add some more details

@anonyein
Copy link

@anonyein anonyein commented on f3f1b3f Oct 5, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rbri https://github.com/requirejs/r.js

File rJs = new File(rhinoJsDir, "r.js");
if (rJs.exists() && rJs.isFile()) {
    try (FileInputStream inputStream = new FileInputStream(rJs)) {
        InputStreamReader envReader = new InputStreamReader(inputStream);
        cx.evaluateReader(scope, envReader,
                "r.js", 1, null);
    } catch (RhinoException re) {
    }
}

cannot be evaluated after this commit with desugar_jdk_libs on Android system.
Maybe desugar_jdk_libs changes "java.util.Optional" to "$j.util.Optional".
I have reverted this commit.

@rbri
Copy link
Collaborator

@rbri rbri commented on f3f1b3f Oct 5, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@anonyein thanks for the hint and the details, can you please open an issue for this to be sure this get lost.

Please sign in to comment.