Skip to content
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.

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

[SPARK-5618][Spark Core][Minor] Optimise utility code. #4396

Closed
wants to merge 1 commit into from

Conversation

fukuo33
Copy link
Contributor

@fukuo33 fukuo33 commented Feb 5, 2015

No description provided.

@AmplabJenkins
Copy link

Can one of the admins verify this patch?

@@ -1067,9 +1067,9 @@ private[spark] object Utils extends Logging {
// finding the call site of a method.
val SPARK_CORE_CLASS_REGEX =
"""^org\.apache\.spark(\.api\.java)?(\.util)?(\.rdd)?(\.broadcast)?\.[A-Z]""".r
val SCALA_CLASS_REGEX = """^scala""".r
val SCALA_CLASS = "scala"
Copy link
Member

Choose a reason for hiding this comment

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

LGTM but could this be called SCALA_CORE_CLASS_PREFIX? why is a lazy val needed? doesn't seem worth it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@srowen Thank you.

The reason for using a "lazy val" is as follows.

The function || takes two Boolean arguments, but only evaluates the second argument if the first is false:

scala> true || { println("!!"); false }
res0: Boolean = true

If "isSparkCoreClass" is true, "className.startsWith (SCALA_CORE_CLASS_PREFIX)" does not need to be evaluated.

Copy link
Member

Choose a reason for hiding this comment

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

Yes I understand that effect, but it's a trivial condition to check, so don't think it overcomes the overhead of the generated code to implement "lazy". It doesn't matter; it's merged.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I got it, thank you!

@andrewor14
Copy link
Contributor

Ok I'm merging this into master and 1.3 with Sean's comments. Thanks @fukuo33

@asfgit asfgit closed this in 4cdb26c Feb 6, 2015
asfgit pushed a commit that referenced this pull request Feb 6, 2015
Author: Makoto Fukuhara <fukuo33@gmail.com>

Closes #4396 from fukuo33/fix-unnecessary-regex and squashes the following commits:

cd07fd6 [Makoto Fukuhara] fix unnecessary regex.

(cherry picked from commit 4cdb26c)
Signed-off-by: Andrew Or <andrew@databricks.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants