Skip to content

Conversation

@viirya
Copy link
Member

@viirya viirya commented May 22, 2017

What changes were proposed in this pull request?

WIP

How was this patch tested?

Jenkins test.

Please review http://spark.apache.org/contributing.html before opening a pull request.

@viirya
Copy link
Member Author

viirya commented May 22, 2017

I am not sure if I am missing something. But looks like we serialize the object in TorrentBroadcast? If I understand it wrongly, I'd close this.

cc @cloud-fan @hvanhovell

@SparkQA
Copy link

SparkQA commented May 22, 2017

Test build #77168 has started for PR 18055 at commit a63df74.

* @param id A unique identifier for the broadcast variable.
*/
private[spark] class TorrentBroadcast[T: ClassTag](obj: T, id: Long)
private[spark] class TorrentBroadcast[T: ClassTag](@transient val obj: T, id: Long)
Copy link
Contributor

Choose a reason for hiding this comment

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

For the old code, obj is not a field, it is just a ctor argument

Copy link
Member Author

@viirya viirya May 22, 2017

Choose a reason for hiding this comment

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

I think the compiler will automatically create a field for it? No?

Copy link
Member Author

Choose a reason for hiding this comment

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

OK. I see. As there is no other places except val numBlocks: Int = writeBlocks(obj) use it, there is no field created for it.

@viirya viirya closed this May 22, 2017
@viirya viirya deleted the transient-broadcast-obj branch May 22, 2017 06:27
@AmplabJenkins
Copy link

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/77168/
Test FAILed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants