Skip to content

[SPARK-38249][CORE][GRAPHX] Cleanup unused private methods/fields#35566

Closed
LuciferYang wants to merge 4 commits intoapache:masterfrom
LuciferYang:never-used
Closed

[SPARK-38249][CORE][GRAPHX] Cleanup unused private methods/fields#35566
LuciferYang wants to merge 4 commits intoapache:masterfrom
LuciferYang:never-used

Conversation

@LuciferYang
Copy link
Contributor

What changes were proposed in this pull request?

This pr aims to cleanup unused ·private methods/fields.

Why are the changes needed?

Code clean.

Does this PR introduce any user-facing change?

No.

How was this patch tested?

Pass GA

@LuciferYang LuciferYang changed the title [SPARK-38249][CORE][GRAPHX] Cleanup unused private methods/fields [WIP][SPARK-38249][CORE][GRAPHX] Cleanup unused private methods/fields Feb 18, 2022
@LuciferYang LuciferYang marked this pull request as draft February 18, 2022 05:21
private val forwardMessageThread =
ThreadUtils.newDaemonSingleThreadScheduledExecutor("master-forward-message-thread")

private val hadoopConf = SparkHadoopUtil.get.newConfiguration(conf)
Copy link
Contributor Author

@LuciferYang LuciferYang Feb 18, 2022

Choose a reason for hiding this comment

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

SPARK-2261 add hadoopConf and used by rebuildSparkUI in the pass, SPARK-12299 Remove history serving functionality from Master and hadoopConf become unused

// After onStart, webUi will be set
private var webUi: MasterWebUI = null

private val masterPublicAddress = {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

masterPublicAddress add in Use external addresses in standalone WebUI on EC2. and never used after SPARK-33774

var message: String = null

// For JSON deserialization
private def setAction(a: String): Unit = { }
Copy link
Contributor Author

Choose a reason for hiding this comment

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

SPARK-5388 add this field but it never seems to be called

Copy link
Member

Choose a reason for hiding this comment

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

I'm worried that the JSON deserializer needs this, per comment


// If this CoarseGrainedExecutorBackend is changed to support multiple threads, then this may need
// to be changed so that we don't share the serializer instance across threads
private[this] val ser: SerializerInstance = env.closureSerializer.newInstance()
Copy link
Contributor Author

Choose a reason for hiding this comment

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

SPARK-3386 move ser out of receive method, it used for deserialize of TaskDescription, but SPARK-17931 change to use TaskDescription do this work

}

private var havePair = false
private var recordsSinceMetricsUpdate = 0
Copy link
Contributor Author

Choose a reason for hiding this comment

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

SPARK-2621 add recordsSinceMetricsUpdate for task metrics and after SPARK-4092 it never used

val resourcePlugins = Utils.loadExtensions(classOf[ResourceDiscoveryPlugin], pluginClasses,
sparkConf)
// apply each plugin until one of them returns the information for this resource
var riOption: Optional[ResourceInformation] = Optional.empty()
Copy link
Contributor Author

Choose a reason for hiding this comment

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

SPARK-30689 add riOption but it was not used because there is a definition with the same name in resourcePlugins.foreach loop

}

/** Test whether the closure accesses the attribute with name `attrName`. */
private def accessesVertexAttr(closure: AnyRef, attrName: String): Boolean = {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Handle ClassNotFoundException from ByteCodeUtils introduce this method and it used by mapReduceTriplets, Code cleaning to improve readability. clean up mapReduceTriplets from GraphImpl.scala, mapReduceTriplets is no longer used

@LuciferYang LuciferYang changed the title [WIP][SPARK-38249][CORE][GRAPHX] Cleanup unused private methods/fields [SPARK-38249][CORE][GRAPHX] Cleanup unused private methods/fields Feb 18, 2022
@LuciferYang LuciferYang marked this pull request as ready for review February 18, 2022 07:12
Copy link
Member

@srowen srowen left a comment

Choose a reason for hiding this comment

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

The rest seems OK pending tests

var message: String = null

// For JSON deserialization
private def setAction(a: String): Unit = { }
Copy link
Member

Choose a reason for hiding this comment

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

I'm worried that the JSON deserializer needs this, per comment

@LuciferYang
Copy link
Contributor Author

I'm worried that the JSON deserializer needs this, per comment

Ok, b2f2f35 revert change of SubmitRestProtocolMessage

@srowen
Copy link
Member

srowen commented Feb 19, 2022

Merged to master

@srowen srowen closed this in 789a510 Feb 19, 2022
@LuciferYang
Copy link
Contributor Author

thanks

dongjoon-hyun pushed a commit that referenced this pull request Aug 4, 2023
### What changes were proposed in this pull request?
`BytecodeUtils` and `BytecodeUtilsSuite` introduced in [Added the BytecodeUtils class for analyzing bytecode](ae12d16).

#23098 deleted the `BytecodeUtilsSuite`, and after #35566, `BytecodeUtils` is no longer used.

So this pr remove `BytecodeUtils` from `graphx` module.

### Why are the changes needed?
Clean up unnecessary code.

### Does this PR introduce _any_ user-facing change?
No

### How was this patch tested?
Pass GitHub Actions

Closes #42343 from LuciferYang/SPARK-44674.

Authored-by: yangjie01 <yangjie01@baidu.com>
Signed-off-by: Dongjoon Hyun <dongjoon@apache.org>
@LuciferYang LuciferYang deleted the never-used branch October 22, 2023 07:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants