Skip to content

[SPARK-20871][SQL] limit logging of Janino code #18658

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
pjfanning wants to merge 6 commits into from
Closed

[SPARK-20871][SQL] limit logging of Janino code #18658

Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
803166c
[SPARK-20871] only log Janino code at debug level
Jul 17, 2017
52b20f3
[SPARK-20871][SQL] log Janino code at info level (but include a max o…
Jul 17, 2017
2997ded
Explicitly reference the maxLines param
Jul 21, 2017
252f8ea
[SPARK-20871][SQL] have just one format function with a default appli…
Jul 21, 2017
c42dc46
[SPARK-20871][SQL] add SQLConf internal property: spark.sql.codegen.l…
Jul 22, 2017
a795db2
[SPARK-20871][SQL] add check on SQLConf internal property: spark.sql.…
Jul 23, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 3 additions & 5 deletions ...lyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/codegen/CodeGenerator.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1037,24 +1037,22 @@ object CodeGenerator extends Logging {
))
evaluator.setExtendedClass(classOf[GeneratedClass])

lazy val formatted = CodeFormatter.format(code)

logDebug({
// Only add extra debugging info to byte code when we are going to print the source code.
evaluator.setDebuggingInformation(true, true, false)
s"\n$formatted"
s"\n${CodeFormatter.format(code)}"
Copy link
Contributor

@ash211 ash211 Jul 17, 2017

Choose a reason for hiding this comment

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

I'd suggest dropping the string concatenation with \n here -- it requires an additional copy of the code to be held in-memory and for errors where the code is too long, this causes unnecessary additional pressure on the heap

Copy link
Member Author

@pjfanning pjfanning Jul 17, 2017

Choose a reason for hiding this comment

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

@ash211 the logging will be less user friendly without the \n and this is debug logging - I am working on an enhancement to the CodeFormatter class for the info level logging discussed earlier with @marmbrus that would allow a max number of lines to be specified

})

try {
evaluator.cook("generated.java", code.body)
recordCompilationStats(evaluator)
} catch {
case e: JaninoRuntimeException =>
val msg = s"failed to compile: $e\n$formatted"
val msg = s"failed to compile: $e"
logError(msg, e)
throw new JaninoRuntimeException(msg, e)
case e: CompileException =>
val msg = s"failed to compile: $e\n$formatted"
val msg = s"failed to compile: $e"
logError(msg, e)
throw new CompileException(msg, e.getLocation)
}
Expand Down