You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by techaddict <gi...@git.apache.org> on 2016/05/04 22:42:50 UTC

[GitHub] spark pull request: [SPARK-928][CORE] Add support for Unsafe-based...

GitHub user techaddict opened a pull request:

    https://github.com/apache/spark/pull/12913

    [SPARK-928][CORE] Add support for Unsafe-based serializer in Kryo

    ## What changes were proposed in this pull request?
    Now since we have migrated to Kryo-3.0.0 in https://issues.apache.org/jira/browse/SPARK-11416, we can gives users option to use unsafe SerDer. It can turned by setting `spark.kryo.useUnsafe` to `true`
    
    ## How was this patch tested?
    Ran existing tests

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/techaddict/spark SPARK-928

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/spark/pull/12913.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #12913
    
----
commit 5641367687760fcc2799a4cfe849d4bc9726f36b
Author: Sandeep Singh <sa...@origamilogic.com>
Date:   2016-05-01T16:11:33Z

    [SPARK-928][CORE] Add support for Unsafe-based serializer in Kryo

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #12913: [SPARK-928][CORE] Add support for Unsafe-based se...

Posted by rxin <gi...@git.apache.org>.
Github user rxin commented on a diff in the pull request:

    https://github.com/apache/spark/pull/12913#discussion_r84570771
  
    --- Diff: core/src/test/scala/org/apache/spark/serializer/KryoBenchmark.scala ---
    @@ -0,0 +1,133 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more
    + * contributor license agreements.  See the NOTICE file distributed with
    + * this work for additional information regarding copyright ownership.
    + * The ASF licenses this file to You under the Apache License, Version 2.0
    + * (the "License"); you may not use this file except in compliance with
    + * the License.  You may obtain a copy of the License at
    + *
    + *    http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +package org.apache.spark.serializer
    +
    +import scala.reflect.ClassTag
    +import scala.util.Random
    +
    +import org.apache.spark.{SparkConf, SparkFunSuite}
    +import org.apache.spark.serializer.KryoTest._
    +import org.apache.spark.util.Benchmark
    +
    +class KryoBenchmark extends SparkFunSuite {
    +  val benchmark = new Benchmark("Benchmark Kryo Unsafe vs safe Serialization", 1024 * 1024 * 15, 10)
    +
    +  ignore(s"Benchmark Kryo Unsafe vs safe Serialization") {
    +    Seq (true, false).foreach (runBenchmark)
    +    benchmark.run()
    +
    +    // scalastyle:off
    +    /*
    +      Benchmark Kryo Unsafe vs safe Serialization: Best/Avg Time(ms)    Rate(M/s)   Per Row(ns)   Relative
    +      ------------------------------------------------------------------------------------------------
    +      basicTypes: Int unsafe:true                    160 /  178         98.5          10.1       1.0X
    +      basicTypes: Long unsafe:true                   210 /  218         74.9          13.4       0.8X
    +      basicTypes: Float unsafe:true                  203 /  213         77.5          12.9       0.8X
    +      basicTypes: Double unsafe:true                 226 /  235         69.5          14.4       0.7X
    +      Array: Int unsafe:true                        1087 / 1101         14.5          69.1       0.1X
    +      Array: Long unsafe:true                       2758 / 2844          5.7         175.4       0.1X
    +      Array: Float unsafe:true                      1511 / 1552         10.4          96.1       0.1X
    +      Array: Double unsafe:true                     2942 / 2972          5.3         187.0       0.1X
    +      Map of string->Double unsafe:true             2645 / 2739          5.9         168.2       0.1X
    +      basicTypes: Int unsafe:false                   211 /  218         74.7          13.4       0.8X
    +      basicTypes: Long unsafe:false                  247 /  253         63.6          15.7       0.6X
    +      basicTypes: Float unsafe:false                 211 /  216         74.5          13.4       0.8X
    +      basicTypes: Double unsafe:false                227 /  233         69.2          14.4       0.7X
    +      Array: Int unsafe:false                       3012 / 3032          5.2         191.5       0.1X
    +      Array: Long unsafe:false                      4463 / 4515          3.5         283.8       0.0X
    +      Array: Float unsafe:false                     2788 / 2868          5.6         177.2       0.1X
    +      Array: Double unsafe:false                    3558 / 3752          4.4         226.2       0.0X
    +      Map of string->Double unsafe:false            2806 / 2933          5.6         178.4       0.1X
    +    */
    +    // scalastyle:on
    +  }
    +
    +  private def runBenchmark(useUnsafe: Boolean): Unit = {
    +    def addBenchmark(name: String, values: Long)(f: => Long): Unit = {
    +      benchmark.addCase(s"$name unsafe:$useUnsafe") { _ =>
    +        f
    +      }
    +    }
    +
    +    def check[T: ClassTag](t: T, ser: SerializerInstance): Int = {
    +      if (ser.deserialize[T](ser.serialize(t)) === t) 1 else 0
    +    }
    +
    +    val N1 = 1000000
    +    def basicTypes[T: ClassTag](name: String, fn: () => T): Unit = {
    --- End diff --
    
    the use of call by name for fn is really confusing, especially the way you are calling it.



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #12913: [SPARK-928][CORE] Add support for Unsafe-based se...

Posted by mateiz <gi...@git.apache.org>.
Github user mateiz commented on a diff in the pull request:

    https://github.com/apache/spark/pull/12913#discussion_r83971396
  
    --- Diff: core/src/main/scala/org/apache/spark/serializer/KryoSerializer.scala ---
    @@ -78,8 +79,14 @@ class KryoSerializer(conf: SparkConf)
         .filter(!_.isEmpty)
     
       private val avroSchemas = conf.getAvroSchema
    +  private val useUnsafe = conf.getBoolean("spark.kryo.useUnsafe", false)
    --- End diff --
    
    Nit: I'd call this spark.kryo.unsafe to make it easier to remember


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #12913: [SPARK-928][CORE] Add support for Unsafe-based serialize...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/12913
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/67282/
    Test PASSed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-928][CORE] Add support for Unsafe-based...

Posted by rxin <gi...@git.apache.org>.
Github user rxin commented on a diff in the pull request:

    https://github.com/apache/spark/pull/12913#discussion_r62137084
  
    --- Diff: core/src/test/scala/org/apache/spark/scheduler/KryoBenchmark.scala ---
    @@ -0,0 +1,125 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more
    + * contributor license agreements.  See the NOTICE file distributed with
    + * this work for additional information regarding copyright ownership.
    + * The ASF licenses this file to You under the Apache License, Version 2.0
    + * (the "License"); you may not use this file except in compliance with
    + * the License.  You may obtain a copy of the License at
    + *
    + *    http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +package org.apache.spark.scheduler
    +
    +import scala.reflect.ClassTag
    +import scala.util.Random
    +
    +import org.apache.spark.{SharedSparkContext, SparkFunSuite}
    +import org.apache.spark.serializer.KryoSerializer
    +import org.apache.spark.serializer.KryoTest._
    +import org.apache.spark.util.Benchmark
    +
    +class KryoBenchmark extends SparkFunSuite with SharedSparkContext {
    +  conf.set("spark.serializer", "org.apache.spark.serializer.KryoSerializer")
    +  conf.set("spark.kryo.registrator", classOf[MyRegistrator].getName)
    +
    +  val benchmark = new Benchmark("Benchmark Kryo Unsafe vs safe Serialization", 1024 * 1024 * 15, 10)
    +
    +  test(s"Benchmark Kryo Unsafe vs safe Serialization") {
    +    Seq (false, true).foreach (runBenchmark)
    +    benchmark.run()
    +
    +    // scalastyle:off
    +    /*
    +      Java HotSpot(TM) 64-Bit Server VM 1.8.0_60-b27 on Mac OS X 10.11.4
    +      Intel(R) Core(TM) i7-4770HQ CPU @ 2.20GHz
    +
    +      Benchmark Kryo Unsafe vs safe Serialization: Best/Avg Time(ms)    Rate(M/s)   Per Row(ns)   Relative
    +      ------------------------------------------------------------------------------------------------
    +      basicTypes: Int unsafe:false                     2 /    4       8988.0           0.1       1.0X
    --- End diff --
    
    this benchmark is probably wrong. these numbers don't make sense.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #12913: [SPARK-928][CORE] Add support for Unsafe-based se...

Posted by rxin <gi...@git.apache.org>.
Github user rxin commented on a diff in the pull request:

    https://github.com/apache/spark/pull/12913#discussion_r84570758
  
    --- Diff: core/src/test/scala/org/apache/spark/serializer/KryoBenchmark.scala ---
    @@ -0,0 +1,133 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more
    + * contributor license agreements.  See the NOTICE file distributed with
    + * this work for additional information regarding copyright ownership.
    + * The ASF licenses this file to You under the Apache License, Version 2.0
    + * (the "License"); you may not use this file except in compliance with
    + * the License.  You may obtain a copy of the License at
    + *
    + *    http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +package org.apache.spark.serializer
    +
    +import scala.reflect.ClassTag
    +import scala.util.Random
    +
    +import org.apache.spark.{SparkConf, SparkFunSuite}
    +import org.apache.spark.serializer.KryoTest._
    +import org.apache.spark.util.Benchmark
    +
    +class KryoBenchmark extends SparkFunSuite {
    +  val benchmark = new Benchmark("Benchmark Kryo Unsafe vs safe Serialization", 1024 * 1024 * 15, 10)
    +
    +  ignore(s"Benchmark Kryo Unsafe vs safe Serialization") {
    +    Seq (true, false).foreach (runBenchmark)
    +    benchmark.run()
    +
    +    // scalastyle:off
    +    /*
    +      Benchmark Kryo Unsafe vs safe Serialization: Best/Avg Time(ms)    Rate(M/s)   Per Row(ns)   Relative
    +      ------------------------------------------------------------------------------------------------
    +      basicTypes: Int unsafe:true                    160 /  178         98.5          10.1       1.0X
    +      basicTypes: Long unsafe:true                   210 /  218         74.9          13.4       0.8X
    +      basicTypes: Float unsafe:true                  203 /  213         77.5          12.9       0.8X
    +      basicTypes: Double unsafe:true                 226 /  235         69.5          14.4       0.7X
    +      Array: Int unsafe:true                        1087 / 1101         14.5          69.1       0.1X
    +      Array: Long unsafe:true                       2758 / 2844          5.7         175.4       0.1X
    +      Array: Float unsafe:true                      1511 / 1552         10.4          96.1       0.1X
    +      Array: Double unsafe:true                     2942 / 2972          5.3         187.0       0.1X
    +      Map of string->Double unsafe:true             2645 / 2739          5.9         168.2       0.1X
    +      basicTypes: Int unsafe:false                   211 /  218         74.7          13.4       0.8X
    +      basicTypes: Long unsafe:false                  247 /  253         63.6          15.7       0.6X
    +      basicTypes: Float unsafe:false                 211 /  216         74.5          13.4       0.8X
    +      basicTypes: Double unsafe:false                227 /  233         69.2          14.4       0.7X
    +      Array: Int unsafe:false                       3012 / 3032          5.2         191.5       0.1X
    +      Array: Long unsafe:false                      4463 / 4515          3.5         283.8       0.0X
    +      Array: Float unsafe:false                     2788 / 2868          5.6         177.2       0.1X
    +      Array: Double unsafe:false                    3558 / 3752          4.4         226.2       0.0X
    +      Map of string->Double unsafe:false            2806 / 2933          5.6         178.4       0.1X
    +    */
    +    // scalastyle:on
    +  }
    +
    +  private def runBenchmark(useUnsafe: Boolean): Unit = {
    +    def addBenchmark(name: String, values: Long)(f: => Long): Unit = {
    --- End diff --
    
    I find this function is over abstracted and actually makes it more confusing what's going on. I'd just remove it since it is almost no-op.



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #12913: [SPARK-928][CORE] Add support for Unsafe-based serialize...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/12913
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/67376/
    Test PASSed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-928][CORE] Add support for Unsafe-based...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/12913#issuecomment-217117020
  
    Merged build finished. Test PASSed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-928][CORE] Add support for Unsafe-based...

Posted by rxin <gi...@git.apache.org>.
Github user rxin commented on a diff in the pull request:

    https://github.com/apache/spark/pull/12913#discussion_r62137117
  
    --- Diff: core/src/test/scala/org/apache/spark/scheduler/KryoBenchmark.scala ---
    @@ -0,0 +1,125 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more
    + * contributor license agreements.  See the NOTICE file distributed with
    + * this work for additional information regarding copyright ownership.
    + * The ASF licenses this file to You under the Apache License, Version 2.0
    + * (the "License"); you may not use this file except in compliance with
    + * the License.  You may obtain a copy of the License at
    + *
    + *    http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +package org.apache.spark.scheduler
    +
    +import scala.reflect.ClassTag
    +import scala.util.Random
    +
    +import org.apache.spark.{SharedSparkContext, SparkFunSuite}
    +import org.apache.spark.serializer.KryoSerializer
    +import org.apache.spark.serializer.KryoTest._
    +import org.apache.spark.util.Benchmark
    +
    +class KryoBenchmark extends SparkFunSuite with SharedSparkContext {
    +  conf.set("spark.serializer", "org.apache.spark.serializer.KryoSerializer")
    +  conf.set("spark.kryo.registrator", classOf[MyRegistrator].getName)
    +
    +  val benchmark = new Benchmark("Benchmark Kryo Unsafe vs safe Serialization", 1024 * 1024 * 15, 10)
    +
    +  test(s"Benchmark Kryo Unsafe vs safe Serialization") {
    +    Seq (false, true).foreach (runBenchmark)
    +    benchmark.run()
    +
    +    // scalastyle:off
    +    /*
    +      Java HotSpot(TM) 64-Bit Server VM 1.8.0_60-b27 on Mac OS X 10.11.4
    +      Intel(R) Core(TM) i7-4770HQ CPU @ 2.20GHz
    +
    +      Benchmark Kryo Unsafe vs safe Serialization: Best/Avg Time(ms)    Rate(M/s)   Per Row(ns)   Relative
    +      ------------------------------------------------------------------------------------------------
    +      basicTypes: Int unsafe:false                     2 /    4       8988.0           0.1       1.0X
    +      basicTypes: Long unsafe:false                    1 /    1      13981.3           0.1       1.6X
    +      basicTypes: Float unsafe:false                   1 /    1      14460.6           0.1       1.6X
    +      basicTypes: Double unsafe:false                  1 /    1      15876.9           0.1       1.8X
    +      Array: Int unsafe:false                         33 /   44        474.8           2.1       0.1X
    +      Array: Long unsafe:false                        18 /   25        888.6           1.1       0.1X
    +      Array: Float unsafe:false                       10 /   16       1627.4           0.6       0.2X
    +      Array: Double unsafe:false                      10 /   13       1523.1           0.7       0.2X
    +      Map of string->Double unsafe:false             413 /  447         38.1          26.3       0.0X
    +      basicTypes: Int unsafe:true                      1 /    1      16402.6           0.1       1.8X
    +      basicTypes: Long unsafe:true                     1 /    1      19732.1           0.1       2.2X
    +      basicTypes: Float unsafe:true                    1 /    1      19752.9           0.1       2.2X
    +      basicTypes: Double unsafe:true                   1 /    1      23111.4           0.0       2.6X
    +      Array: Int unsafe:true                           7 /    8       2239.9           0.4       0.2X
    +      Array: Long unsafe:true                          8 /    9       2000.1           0.5       0.2X
    +      Array: Float unsafe:true                         7 /    8       2191.5           0.5       0.2X
    +      Array: Double unsafe:true                        9 /   10       1841.2           0.5       0.2X
    +      Map of string->Double unsafe:true              387 /  407         40.7          24.6       0.0X
    +    */
    +    // scalastyle:on
    +  }
    +
    +  private def runBenchmark(useUnsafe: Boolean): Unit = {
    +    conf.set("spark.kryo.useUnsafe", useUnsafe.toString)
    +    val ser = new KryoSerializer(conf).newInstance()
    +
    +    def addBenchmark(name: String, values: Long)(f: => Long): Unit = {
    +      benchmark.addCase(s"$name unsafe:$useUnsafe") { iters =>
    +        f + 1
    +      }
    +    }
    +
    +    def check[T: ClassTag](t: T): Int = {
    +      if (ser.deserialize[T](ser.serialize(t)) === t) 1 else 0
    +    }
    +
    +    var N = 5000000
    +    basicTypes("Int", Random.nextInt)
    +    basicTypes("Long", Random.nextLong)
    +    basicTypes("Float", Random.nextFloat)
    +    basicTypes("Double", Random.nextDouble)
    +
    +    N = 100000
    +    basicTypeArray("Int", Random.nextInt)
    +    basicTypeArray("Long", Random.nextLong)
    +    basicTypeArray("Float", Random.nextFloat)
    +    basicTypeArray("Double", Random.nextDouble)
    +
    +    N = 500
    +    addBenchmark("Map of string->Double", N) {
    +      var sum = 0L
    +      for (i <- 1 to N) {
    +        val map = Array.fill(i)((Random.nextString(i/10), Random.nextDouble())).toMap
    +        sum += check(map)
    +      }
    +      sum
    +    }
    +
    +    def basicTypes[T: ClassTag](name: String, fn: () => T): Unit = {
    +      addBenchmark(s"basicTypes: $name", N) {
    +        var sum = 0L
    +        for (i <- 1 to N) {
    --- End diff --
    
    i think the whole loop might've gotten dead code eliminated. also for loop is pretty expensive so you are probably benchmarking the wrong thing here.



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-928][CORE] Add support for Unsafe-based...

Posted by techaddict <gi...@git.apache.org>.
Github user techaddict commented on the pull request:

    https://github.com/apache/spark/pull/12913#issuecomment-217034618
  
    @vectorijk The class Kryo Serializer is not wire-compatible, so IMO we should be ok. Either one uses Unsafe or safe based at a time.
    Comment from the KryoSerializer class
    ```Note that this serializer is not guaranteed to be wire-compatible across different versions of
      Spark. It is intended to be used to serialize/de-serialize data within a single
      Spark application.```


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #12913: [SPARK-928][CORE] Add support for Unsafe-based se...

Posted by mateiz <gi...@git.apache.org>.
Github user mateiz commented on a diff in the pull request:

    https://github.com/apache/spark/pull/12913#discussion_r84570152
  
    --- Diff: core/src/test/scala/org/apache/spark/serializer/UnsafeKryoSerializerSuite.scala ---
    @@ -0,0 +1,28 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more
    + * contributor license agreements.  See the NOTICE file distributed with
    + * this work for additional information regarding copyright ownership.
    + * The ASF licenses this file to You under the Apache License, Version 2.0
    + * (the "License"); you may not use this file except in compliance with
    + * the License.  You may obtain a copy of the License at
    + *
    + *    http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +package org.apache.spark.serializer
    +
    +class UnsafeKryoSerializerSuite extends KryoSerializerSuite {
    +
    +  // This test suite should run all tests in KryoSerializerSuite with kryo unsafe.
    +
    +  override def beforeAll() {
    +    super.beforeAll()
    +    conf.set("spark.kryo.unsafe", "true")
    --- End diff --
    
    Have you checked that this runs in the right order w.r.t. the code that initializes a SparkContext in KryoSerializerSuite / SharedSparkContext? You may want to print sc.conf in a test. Otherwise, it would be better to put conf.set("spark.kryo.unsafe", "true") at the top-level inside this class. SharedSparkContext does create a SparkContext in its beforeAll, but you seem to editing the conf after that is done.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-928][CORE] Add support for Unsafe-based...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/12913#issuecomment-217026635
  
    **[Test build #57811 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/57811/consoleFull)** for PR 12913 at commit [`5641367`](https://github.com/apache/spark/commit/5641367687760fcc2799a4cfe849d4bc9726f36b).


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-928][CORE] Add support for Unsafe-based...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/12913#issuecomment-217117021
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/57862/
    Test PASSed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #12913: [SPARK-928][CORE] Add support for Unsafe-based serialize...

Posted by techaddict <gi...@git.apache.org>.
Github user techaddict commented on the issue:

    https://github.com/apache/spark/pull/12913
  
    @mateiz updated \U0001f44d 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #12913: [SPARK-928][CORE] Add support for Unsafe-based se...

Posted by mateiz <gi...@git.apache.org>.
Github user mateiz commented on a diff in the pull request:

    https://github.com/apache/spark/pull/12913#discussion_r83971706
  
    --- Diff: core/src/test/scala/org/apache/spark/serializer/KryoSerializerSuite.scala ---
    @@ -75,9 +75,11 @@ class KryoSerializerSuite extends SparkFunSuite with SharedSparkContext {
       }
     
       test("basic types") {
    -    val ser = new KryoSerializer(conf).newInstance()
         def check[T: ClassTag](t: T) {
    -      assert(ser.deserialize[T](ser.serialize(t)) === t)
    +      testBothUnsafeAndSafe(conf) { sparkConf =>
    --- End diff --
    
    Is there any way to refactor this into two Suites, where one extends from the other and has additional setup code (or a setup method that it overrides)? Otherwise we have to remember to add this construct to all tests.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #12913: [SPARK-928][CORE] Add support for Unsafe-based serialize...

Posted by mateiz <gi...@git.apache.org>.
Github user mateiz commented on the issue:

    https://github.com/apache/spark/pull/12913
  
    Looks pretty good overall! I made two small comments but it seems worthwhile to add in and it's not a huge change.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #12913: [SPARK-928][CORE] Add support for Unsafe-based serialize...

Posted by techaddict <gi...@git.apache.org>.
Github user techaddict commented on the issue:

    https://github.com/apache/spark/pull/12913
  
    @holdenk Updated the PR, ready for review again.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #12913: [SPARK-928][CORE] Add support for Unsafe-based serialize...

Posted by maver1ck <gi...@git.apache.org>.
Github user maver1ck commented on the issue:

    https://github.com/apache/spark/pull/12913
  
    Up ?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-928][CORE] Add support for Unsafe-based...

Posted by techaddict <gi...@git.apache.org>.
Github user techaddict commented on the pull request:

    https://github.com/apache/spark/pull/12913#issuecomment-219031508
  
    ping @rxin @JoshRosen @andrewor14 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #12913: [SPARK-928][CORE] Add support for Unsafe-based se...

Posted by rxin <gi...@git.apache.org>.
Github user rxin commented on a diff in the pull request:

    https://github.com/apache/spark/pull/12913#discussion_r84570784
  
    --- Diff: core/src/test/scala/org/apache/spark/serializer/KryoBenchmark.scala ---
    @@ -0,0 +1,133 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more
    + * contributor license agreements.  See the NOTICE file distributed with
    + * this work for additional information regarding copyright ownership.
    + * The ASF licenses this file to You under the Apache License, Version 2.0
    + * (the "License"); you may not use this file except in compliance with
    + * the License.  You may obtain a copy of the License at
    + *
    + *    http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +package org.apache.spark.serializer
    +
    +import scala.reflect.ClassTag
    +import scala.util.Random
    +
    +import org.apache.spark.{SparkConf, SparkFunSuite}
    +import org.apache.spark.serializer.KryoTest._
    +import org.apache.spark.util.Benchmark
    +
    +class KryoBenchmark extends SparkFunSuite {
    +  val benchmark = new Benchmark("Benchmark Kryo Unsafe vs safe Serialization", 1024 * 1024 * 15, 10)
    +
    +  ignore(s"Benchmark Kryo Unsafe vs safe Serialization") {
    +    Seq (true, false).foreach (runBenchmark)
    +    benchmark.run()
    +
    +    // scalastyle:off
    +    /*
    +      Benchmark Kryo Unsafe vs safe Serialization: Best/Avg Time(ms)    Rate(M/s)   Per Row(ns)   Relative
    +      ------------------------------------------------------------------------------------------------
    +      basicTypes: Int unsafe:true                    160 /  178         98.5          10.1       1.0X
    +      basicTypes: Long unsafe:true                   210 /  218         74.9          13.4       0.8X
    +      basicTypes: Float unsafe:true                  203 /  213         77.5          12.9       0.8X
    +      basicTypes: Double unsafe:true                 226 /  235         69.5          14.4       0.7X
    +      Array: Int unsafe:true                        1087 / 1101         14.5          69.1       0.1X
    +      Array: Long unsafe:true                       2758 / 2844          5.7         175.4       0.1X
    +      Array: Float unsafe:true                      1511 / 1552         10.4          96.1       0.1X
    +      Array: Double unsafe:true                     2942 / 2972          5.3         187.0       0.1X
    +      Map of string->Double unsafe:true             2645 / 2739          5.9         168.2       0.1X
    +      basicTypes: Int unsafe:false                   211 /  218         74.7          13.4       0.8X
    +      basicTypes: Long unsafe:false                  247 /  253         63.6          15.7       0.6X
    +      basicTypes: Float unsafe:false                 211 /  216         74.5          13.4       0.8X
    +      basicTypes: Double unsafe:false                227 /  233         69.2          14.4       0.7X
    +      Array: Int unsafe:false                       3012 / 3032          5.2         191.5       0.1X
    +      Array: Long unsafe:false                      4463 / 4515          3.5         283.8       0.0X
    +      Array: Float unsafe:false                     2788 / 2868          5.6         177.2       0.1X
    +      Array: Double unsafe:false                    3558 / 3752          4.4         226.2       0.0X
    +      Map of string->Double unsafe:false            2806 / 2933          5.6         178.4       0.1X
    +    */
    +    // scalastyle:on
    +  }
    +
    +  private def runBenchmark(useUnsafe: Boolean): Unit = {
    +    def addBenchmark(name: String, values: Long)(f: => Long): Unit = {
    +      benchmark.addCase(s"$name unsafe:$useUnsafe") { _ =>
    +        f
    +      }
    +    }
    +
    +    def check[T: ClassTag](t: T, ser: SerializerInstance): Int = {
    +      if (ser.deserialize[T](ser.serialize(t)) === t) 1 else 0
    +    }
    +
    +    val N1 = 1000000
    +    def basicTypes[T: ClassTag](name: String, fn: () => T): Unit = {
    +      lazy val ser = createSerializer(useUnsafe)
    +      addBenchmark(s"basicTypes: $name", N1) {
    +        var sum = 0L
    +        var i = 0
    +        while (i < N1) {
    +          sum += check(fn(), ser)
    +          i += 1
    +        }
    +        sum
    +      }
    +    }
    +    basicTypes("Int", Random.nextInt)
    +    basicTypes("Long", Random.nextLong)
    +    basicTypes("Float", Random.nextFloat)
    +    basicTypes("Double", Random.nextDouble)
    +
    +    val N2 = 10000
    +    def basicTypeArray[T: ClassTag](name: String, fn: () => T): Unit = {
    +      lazy val ser = createSerializer(useUnsafe)
    +      addBenchmark(s"Array: $name", N2) {
    +        var sum = 0L
    +        var i = 0
    +        while (i < N2) {
    +          val arr = Array.fill[T](i)(fn())
    +          sum += check(arr, ser)
    +          i += 1
    +        }
    +        sum
    +      }
    +    }
    +    basicTypeArray("Int", Random.nextInt)
    +    basicTypeArray("Long", Random.nextLong)
    +    basicTypeArray("Float", Random.nextFloat)
    +    basicTypeArray("Double", Random.nextDouble)
    +
    +    {
    +      val N3 = 1000
    +      lazy val ser = createSerializer(useUnsafe)
    +      addBenchmark("Map of string->Double", N3) {
    +        var sum = 0L
    +        var i = 0
    +        while (i < N3) {
    +          val map = Array.fill(i)((Random.nextString(i / 10), Random.nextDouble())).toMap
    --- End diff --
    
    here too ...



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #12913: [SPARK-928][CORE] Add support for Unsafe-based se...

Posted by rxin <gi...@git.apache.org>.
Github user rxin commented on a diff in the pull request:

    https://github.com/apache/spark/pull/12913#discussion_r84570741
  
    --- Diff: core/src/main/scala/org/apache/spark/serializer/KryoSerializer.scala ---
    @@ -248,8 +266,9 @@ class KryoDeserializationStream(
       }
     }
     
    -private[spark] class KryoSerializerInstance(ks: KryoSerializer) extends SerializerInstance {
    -
    +private[spark] class KryoSerializerInstance(
    --- End diff --
    
    ```
    private[spark] class KryoSerializerInstance(ks: KryoSerializer, useUnsafe: Boolean)
      extends SerializerInstance {
    ```


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #12913: [SPARK-928][CORE] Add support for Unsafe-based serialize...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/12913
  
    **[Test build #67282 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/67282/consoleFull)** for PR 12913 at commit [`b9c81a5`](https://github.com/apache/spark/commit/b9c81a5c07797f4d975c2b5a741c497f98235ab5).
     * This patch passes all tests.
     * This patch merges cleanly.
     * This patch adds no public classes.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #12913: [SPARK-928][CORE] Add support for Unsafe-based serialize...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/12913
  
    **[Test build #67377 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/67377/consoleFull)** for PR 12913 at commit [`3977ccb`](https://github.com/apache/spark/commit/3977ccb2f58f7bf23d0bd4807d615c38939d6342).
     * This patch **fails Spark unit tests**.
     * This patch merges cleanly.
     * This patch adds no public classes.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #12913: [SPARK-928][CORE] Add support for Unsafe-based se...

Posted by techaddict <gi...@git.apache.org>.
Github user techaddict commented on a diff in the pull request:

    https://github.com/apache/spark/pull/12913#discussion_r84570678
  
    --- Diff: core/src/test/scala/org/apache/spark/serializer/UnsafeKryoSerializerSuite.scala ---
    @@ -0,0 +1,28 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more
    + * contributor license agreements.  See the NOTICE file distributed with
    + * this work for additional information regarding copyright ownership.
    + * The ASF licenses this file to You under the Apache License, Version 2.0
    + * (the "License"); you may not use this file except in compliance with
    + * the License.  You may obtain a copy of the License at
    + *
    + *    http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +package org.apache.spark.serializer
    +
    +class UnsafeKryoSerializerSuite extends KryoSerializerSuite {
    +
    +  // This test suite should run all tests in KryoSerializerSuite with kryo unsafe.
    +
    +  override def beforeAll() {
    +    super.beforeAll()
    +    conf.set("spark.kryo.unsafe", "true")
    --- End diff --
    
    Ohh yes, fixed and tested \U0001f44d 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-928][CORE] Add support for Unsafe-based...

Posted by techaddict <gi...@git.apache.org>.
Github user techaddict commented on the pull request:

    https://github.com/apache/spark/pull/12913#issuecomment-221259367
  
    cc: @srowen @rxin @JoshRosen 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #12913: [SPARK-928][CORE] Add support for Unsafe-based serialize...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/12913
  
    **[Test build #67197 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/67197/consoleFull)** for PR 12913 at commit [`37ccea4`](https://github.com/apache/spark/commit/37ccea41caf9f9f4bbd4a4cb0fc70c43f23f564f).
     * This patch passes all tests.
     * This patch merges cleanly.
     * This patch adds no public classes.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #12913: [SPARK-928][CORE] Add support for Unsafe-based serialize...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/12913
  
    Merged build finished. Test PASSed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-928][CORE] Add support for Unsafe-based...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/12913#issuecomment-217047701
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/57811/
    Test PASSed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #12913: [SPARK-928][CORE] Add support for Unsafe-based serialize...

Posted by rxin <gi...@git.apache.org>.
Github user rxin commented on the issue:

    https://github.com/apache/spark/pull/12913
  
    LGTM pending Jenkins.



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #12913: [SPARK-928][CORE] Add support for Unsafe-based se...

Posted by holdenk <gi...@git.apache.org>.
Github user holdenk commented on a diff in the pull request:

    https://github.com/apache/spark/pull/12913#discussion_r73431033
  
    --- Diff: core/src/test/scala/org/apache/spark/serializer/KryoSerializerSuite.scala ---
    @@ -399,6 +399,14 @@ class KryoSerializerSuite extends SparkFunSuite with SharedSparkContext {
         assert(!ser2.getAutoReset)
       }
     
    +  private def testBothUnsafeAndSafe(f: SparkConf => Unit): Unit = {
    --- End diff --
    
    At first glance I don't see this function used anywhere I'm assuming this was meant to wrap some of the tests?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-928][CORE] Add support for Unsafe-based...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/12913#issuecomment-217047594
  
    **[Test build #57811 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/57811/consoleFull)** for PR 12913 at commit [`5641367`](https://github.com/apache/spark/commit/5641367687760fcc2799a4cfe849d4bc9726f36b).
     * This patch passes all tests.
     * This patch merges cleanly.
     * This patch adds no public classes.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #12913: [SPARK-928][CORE] Add support for Unsafe-based se...

Posted by techaddict <gi...@git.apache.org>.
Github user techaddict commented on a diff in the pull request:

    https://github.com/apache/spark/pull/12913#discussion_r73454617
  
    --- Diff: core/src/test/scala/org/apache/spark/serializer/KryoSerializerSuite.scala ---
    @@ -399,6 +399,14 @@ class KryoSerializerSuite extends SparkFunSuite with SharedSparkContext {
         assert(!ser2.getAutoReset)
       }
     
    +  private def testBothUnsafeAndSafe(f: SparkConf => Unit): Unit = {
    --- End diff --
    
    Yes will update the pr today.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #12913: [SPARK-928][CORE] Add support for Unsafe-based serialize...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/12913
  
    **[Test build #67378 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/67378/consoleFull)** for PR 12913 at commit [`cdf535e`](https://github.com/apache/spark/commit/cdf535e82c7c482ae030975d53683164854ef588).


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #12913: [SPARK-928][CORE] Add support for Unsafe-based se...

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/spark/pull/12913


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #12913: [SPARK-928][CORE] Add support for Unsafe-based serialize...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/12913
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/63228/
    Test PASSed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #12913: [SPARK-928][CORE] Add support for Unsafe-based serialize...

Posted by techaddict <gi...@git.apache.org>.
Github user techaddict commented on the issue:

    https://github.com/apache/spark/pull/12913
  
    @rxin can you review again, all comments addressed \U0001f44d 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #12913: [SPARK-928][CORE] Add support for Unsafe-based serialize...

Posted by techaddict <gi...@git.apache.org>.
Github user techaddict commented on the issue:

    https://github.com/apache/spark/pull/12913
  
    @mateiz updated the pr \U0001f44d 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #12913: [SPARK-928][CORE] Add support for Unsafe-based serialize...

Posted by holdenk <gi...@git.apache.org>.
Github user holdenk commented on the issue:

    https://github.com/apache/spark/pull/12913
  
    Excellent - lets see so @mateiz was the creator of this issue originally so we should check and see if he is available to review (although I suspect he is rather busy in general). If not we can maybe reach out to @JoshRosen and see if he has any review bandwidth? I'm not familiar enough with Kyro to do a meaningful review myself - but if everyone else is busy I can try and get up to speed on it and see what we can do while we wait for one of the committers to be available.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #12913: [SPARK-928][CORE] Add support for Unsafe-based se...

Posted by mateiz <gi...@git.apache.org>.
Github user mateiz commented on a diff in the pull request:

    https://github.com/apache/spark/pull/12913#discussion_r83972911
  
    --- Diff: core/src/test/scala/org/apache/spark/serializer/KryoSerializerSuite.scala ---
    @@ -75,9 +75,11 @@ class KryoSerializerSuite extends SparkFunSuite with SharedSparkContext {
       }
     
       test("basic types") {
    -    val ser = new KryoSerializer(conf).newInstance()
         def check[T: ClassTag](t: T) {
    -      assert(ser.deserialize[T](ser.serialize(t)) === t)
    +      testBothUnsafeAndSafe(conf) { sparkConf =>
    --- End diff --
    
    (We have a few other examples suites where we just inherit from existing suites, such as SortShuffleSuite vs ShuffleSuite.)


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #12913: [SPARK-928][CORE] Add support for Unsafe-based serialize...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/12913
  
    Merged build finished. Test PASSed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #12913: [SPARK-928][CORE] Add support for Unsafe-based serialize...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/12913
  
    **[Test build #67376 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/67376/consoleFull)** for PR 12913 at commit [`ed2c3a1`](https://github.com/apache/spark/commit/ed2c3a1f7e66e692318995f57a44be11f368e2f7).
     * This patch passes all tests.
     * This patch merges cleanly.
     * This patch adds the following public classes _(experimental)_:
      * `  case class AssignStrategy(partitions: Array[TopicPartition], kafkaParams: ju.Map[String, Object])`
      * `  case class SourceInfo(name: String, schema: StructType, partitionColumns: Seq[String])`
      * `class TextOutputWriter(`


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #12913: [SPARK-928][CORE] Add support for Unsafe-based serialize...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/12913
  
    **[Test build #63228 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/63228/consoleFull)** for PR 12913 at commit [`498a984`](https://github.com/apache/spark/commit/498a98451229b9ca3860d18f7aa7ea78a9ebfc6f).
     * This patch passes all tests.
     * This patch merges cleanly.
     * This patch adds no public classes.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #12913: [SPARK-928][CORE] Add support for Unsafe-based se...

Posted by rxin <gi...@git.apache.org>.
Github user rxin commented on a diff in the pull request:

    https://github.com/apache/spark/pull/12913#discussion_r84570736
  
    --- Diff: core/src/main/scala/org/apache/spark/serializer/KryoSerializer.scala ---
    @@ -186,9 +193,15 @@ class KryoSerializer(conf: SparkConf)
     private[spark]
     class KryoSerializationStream(
         serInstance: KryoSerializerInstance,
    -    outStream: OutputStream) extends SerializationStream {
    +    outStream: OutputStream,
    +    useUnsafe: Boolean) extends SerializationStream {
     
    -  private[this] var output: KryoOutput = new KryoOutput(outStream)
    +  private[this] var output: KryoOutput =
    +    if (useUnsafe) {
    --- End diff --
    
    put this in one line?
    ```
        if (useUnsafe) new KryoUnsafeOutput(outStream) else new KryoOutput(outStream)
    ```


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #12913: [SPARK-928][CORE] Add support for Unsafe-based se...

Posted by rxin <gi...@git.apache.org>.
Github user rxin commented on a diff in the pull request:

    https://github.com/apache/spark/pull/12913#discussion_r84570738
  
    --- Diff: core/src/main/scala/org/apache/spark/serializer/KryoSerializer.scala ---
    @@ -219,9 +232,14 @@ class KryoSerializationStream(
     private[spark]
     class KryoDeserializationStream(
         serInstance: KryoSerializerInstance,
    -    inStream: InputStream) extends DeserializationStream {
    +    inStream: InputStream,
    +    useUnsafe: Boolean) extends DeserializationStream {
     
    -  private[this] var input: KryoInput = new KryoInput(inStream)
    +  private[this] var input: KryoInput = if (useUnsafe) {
    +    new KryoUnsafeInput(inStream)
    --- End diff --
    
    one line here too


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #12913: [SPARK-928][CORE] Add support for Unsafe-based serialize...

Posted by rxin <gi...@git.apache.org>.
Github user rxin commented on the issue:

    https://github.com/apache/spark/pull/12913
  
    Merging in master. Thanks!



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-928][CORE] Add support for Unsafe-based...

Posted by vectorijk <gi...@git.apache.org>.
Github user vectorijk commented on the pull request:

    https://github.com/apache/spark/pull/12913#issuecomment-217031852
  
    @techaddict According to [this](https://github.com/EsotericSoftware/kryo#-disclaimer-about-using-unsafe-based-io-), I am just wondering what will happen when Unsafe-based IO is not compatible with Kryo's Input and Output streams.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #12913: [SPARK-928][CORE] Add support for Unsafe-based serialize...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/12913
  
    Merged build finished. Test PASSed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #12913: [SPARK-928][CORE] Add support for Unsafe-based serialize...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/12913
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/67197/
    Test PASSed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #12913: [SPARK-928][CORE] Add support for Unsafe-based serialize...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/12913
  
    **[Test build #63228 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/63228/consoleFull)** for PR 12913 at commit [`498a984`](https://github.com/apache/spark/commit/498a98451229b9ca3860d18f7aa7ea78a9ebfc6f).


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-928][CORE] Add support for Unsafe-based...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/12913#issuecomment-217100209
  
    **[Test build #57862 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/57862/consoleFull)** for PR 12913 at commit [`112497b`](https://github.com/apache/spark/commit/112497b9f565d9f2a9429ad05e5f49d1b0b285a1).


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #12913: [SPARK-928][CORE] Add support for Unsafe-based serialize...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/12913
  
    **[Test build #67376 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/67376/consoleFull)** for PR 12913 at commit [`ed2c3a1`](https://github.com/apache/spark/commit/ed2c3a1f7e66e692318995f57a44be11f368e2f7).


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #12913: [SPARK-928][CORE] Add support for Unsafe-based se...

Posted by rxin <gi...@git.apache.org>.
Github user rxin commented on a diff in the pull request:

    https://github.com/apache/spark/pull/12913#discussion_r84570727
  
    --- Diff: core/src/main/scala/org/apache/spark/serializer/KryoSerializer.scala ---
    @@ -78,8 +79,14 @@ class KryoSerializer(conf: SparkConf)
         .filter(!_.isEmpty)
     
       private val avroSchemas = conf.getAvroSchema
    +  private val useUnsafe = conf.getBoolean("spark.kryo.unsafe", false)
    --- End diff --
    
    we should document what this means.



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #12913: [SPARK-928][CORE] Add support for Unsafe-based serialize...

Posted by mateiz <gi...@git.apache.org>.
Github user mateiz commented on the issue:

    https://github.com/apache/spark/pull/12913
  
    @techaddict Cool, thanks! Just remembered a couple more things:
    - Can you edit KryoSerializerSuite to set the flag to false? Otherwise we might silently end up with both suites testing on true if we ever change the default.
    - Edit docs/configuration.md to add this new setting to the table about configs (near the other Kryo settings).


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #12913: [SPARK-928][CORE] Add support for Unsafe-based serialize...

Posted by techaddict <gi...@git.apache.org>.
Github user techaddict commented on the issue:

    https://github.com/apache/spark/pull/12913
  
    ping @JoshRosen 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-928][CORE] Add support for Unsafe-based...

Posted by techaddict <gi...@git.apache.org>.
Github user techaddict commented on the pull request:

    https://github.com/apache/spark/pull/12913#issuecomment-217656238
  
    @JoshRosen @rxin can you review this.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #12913: [SPARK-928][CORE] Add support for Unsafe-based serialize...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/12913
  
    **[Test build #67282 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/67282/consoleFull)** for PR 12913 at commit [`b9c81a5`](https://github.com/apache/spark/commit/b9c81a5c07797f4d975c2b5a741c497f98235ab5).


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #12913: [SPARK-928][CORE] Add support for Unsafe-based serialize...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/12913
  
    Merged build finished. Test PASSed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #12913: [SPARK-928][CORE] Add support for Unsafe-based serialize...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/12913
  
    **[Test build #67377 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/67377/consoleFull)** for PR 12913 at commit [`3977ccb`](https://github.com/apache/spark/commit/3977ccb2f58f7bf23d0bd4807d615c38939d6342).


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #12913: [SPARK-928][CORE] Add support for Unsafe-based serialize...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/12913
  
    Merged build finished. Test FAILed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #12913: [SPARK-928][CORE] Add support for Unsafe-based serialize...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/12913
  
    Test FAILed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/67377/
    Test FAILed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-928][CORE] Add support for Unsafe-based...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/12913#issuecomment-217047700
  
    Merged build finished. Test PASSed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #12913: [SPARK-928][CORE] Add support for Unsafe-based serialize...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/12913
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/67378/
    Test PASSed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #12913: [SPARK-928][CORE] Add support for Unsafe-based se...

Posted by rxin <gi...@git.apache.org>.
Github user rxin commented on a diff in the pull request:

    https://github.com/apache/spark/pull/12913#discussion_r84570704
  
    --- Diff: core/src/test/scala/org/apache/spark/serializer/UnsafeKryoSerializerSuite.scala ---
    @@ -0,0 +1,28 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more
    + * contributor license agreements.  See the NOTICE file distributed with
    + * this work for additional information regarding copyright ownership.
    + * The ASF licenses this file to You under the Apache License, Version 2.0
    + * (the "License"); you may not use this file except in compliance with
    + * the License.  You may obtain a copy of the License at
    + *
    + *    http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +package org.apache.spark.serializer
    +
    +class UnsafeKryoSerializerSuite extends KryoSerializerSuite {
    +
    +  // This test suite should run all tests in KryoSerializerSuite with kryo unsafe.
    +
    +  override def beforeAll() {
    --- End diff --
    
    don't you need to reset this back in afterAll?



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-928][CORE] Add support for Unsafe-based...

Posted by techaddict <gi...@git.apache.org>.
Github user techaddict commented on the pull request:

    https://github.com/apache/spark/pull/12913#issuecomment-217100211
  
    @rxin figured out the problem, was using `var N` and its value was changed before the benchmark ran so each benchmark was running effectvely for `N=5000`, thus giving an impression of dead code elimination.
    Plus I was passing function to kryo instead of value in one of the cases, so that was also just wrong.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #12913: [SPARK-928][CORE] Add support for Unsafe-based serialize...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/12913
  
    Merged build finished. Test PASSed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #12913: [SPARK-928][CORE] Add support for Unsafe-based serialize...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/12913
  
    **[Test build #67197 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/67197/consoleFull)** for PR 12913 at commit [`37ccea4`](https://github.com/apache/spark/commit/37ccea41caf9f9f4bbd4a4cb0fc70c43f23f564f).


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-928][CORE] Add support for Unsafe-based...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/12913#issuecomment-217116869
  
    **[Test build #57862 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/57862/consoleFull)** for PR 12913 at commit [`112497b`](https://github.com/apache/spark/commit/112497b9f565d9f2a9429ad05e5f49d1b0b285a1).
     * This patch passes all tests.
     * This patch merges cleanly.
     * This patch adds the following public classes _(experimental)_:
      * `class KryoBenchmark extends SparkFunSuite `


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #12913: [SPARK-928][CORE] Add support for Unsafe-based serialize...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/12913
  
    **[Test build #67378 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/67378/consoleFull)** for PR 12913 at commit [`cdf535e`](https://github.com/apache/spark/commit/cdf535e82c7c482ae030975d53683164854ef588).
     * This patch passes all tests.
     * This patch merges cleanly.
     * This patch adds no public classes.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org