You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by Geek-He <gi...@git.apache.org> on 2017/08/23 03:14:41 UTC

[GitHub] spark pull request #19025: [SPARK-21813][core] Modify TaskMemoryManager.MAXI...

GitHub user Geek-He opened a pull request:

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

    [SPARK-21813][core] Modify TaskMemoryManager.MAXIMUM_PAGE_SIZE_BYTES comments

    ## What changes were proposed in this pull request?
    The variable "TaskMemoryManager.MAXIMUM_PAGE_SIZE_BYTES" comment error, It shouldn't be 2^32-1, should be 2^31-1, That means the maximum value of int.
    
    
    ## How was this patch tested?
    Existing test cases


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

    $ git pull https://github.com/Geek-He/spark 08_23_comments

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

    https://github.com/apache/spark/pull/19025.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 #19025
    
----
commit 0f21a86004876da1319c779ca4cf511cf9b3bd05
Author: he.qiao <he...@zte.com.cn>
Date:   2017-08-23T03:00:15Z

    [core] Modify TaskMemoryManager.MAXIMUM_PAGE_SIZE_BYTES comments

----


---
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 #19025: [SPARK-21813][core] Modify TaskMemoryManager.MAXI...

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

    https://github.com/apache/spark/pull/19025#discussion_r135195213
  
    --- Diff: core/src/main/java/org/apache/spark/memory/TaskMemoryManager.java ---
    @@ -74,7 +74,7 @@
        * Maximum supported data page size (in bytes). In principle, the maximum addressable page size is
        * (1L &lt;&lt; OFFSET_BITS) bytes, which is 2+ petabytes. However, the on-heap allocator's
        * maximum page size is limited by the maximum amount of data that can be stored in a long[]
    -   * array, which is (2^32 - 1) * 8 bytes (or 16 gigabytes). Therefore, we cap this at 16 gigabytes.
    +   * array, which is (2^31 - 1) * 8 bytes (or 16 gigabytes). Therefore, we cap this at 16 gigabytes.
    --- End diff --
    
    Why be imprecise, if you're bothering to correct other things here? it's closer to 17. It's actually greater than 17. 


---
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 #19025: [SPARK-21813][core] Modify TaskMemoryManager.MAXI...

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

    https://github.com/apache/spark/pull/19025#discussion_r134664137
  
    --- Diff: core/src/main/java/org/apache/spark/memory/TaskMemoryManager.java ---
    @@ -53,7 +53,7 @@
      * retrieve the base object.
      * <p>
      * This allows us to address 8192 pages. In on-heap mode, the maximum page size is limited by the
    - * maximum size of a long[] array, allowing us to address 8192 * 2^32 * 8 bytes, which is
    + * maximum size of a long[] array, allowing us to address 8192 * 2^31 * 8 bytes, which is
    --- End diff --
    
    Yes, I'm ignoring the minus one, I'll change it right away. But I didn't understand. Why not 16GB, (2^31-1)*8 = 17179869176 Bytes = 17179869176/1024/1024/1024 = 16GB. so, the amount of memory is more like 128TB(16GB * 8192/1024 = 128TB), not 140TB. I think 1024GB is 1TB, not 1000GB
    (To be honest,  I was puzzled how 35TB was calculated,  and now I know that it was a miscalculation)


---
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 #19025: [SPARK-21813][core] Modify TaskMemoryManager.MAXI...

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

    https://github.com/apache/spark/pull/19025#discussion_r134664599
  
    --- Diff: core/src/main/java/org/apache/spark/memory/TaskMemoryManager.java ---
    @@ -74,7 +74,7 @@
        * Maximum supported data page size (in bytes). In principle, the maximum addressable page size is
        * (1L &lt;&lt; OFFSET_BITS) bytes, which is 2+ petabytes. However, the on-heap allocator's
        * maximum page size is limited by the maximum amount of data that can be stored in a long[]
    -   * array, which is (2^32 - 1) * 8 bytes (or 16 gigabytes). Therefore, we cap this at 16 gigabytes.
    +   * array, which is (2^31 - 1) * 8 bytes (or 16 gigabytes). Therefore, we cap this at 16 gigabytes.
    --- End diff --
    
    (2^31-1)*8 = 17179869176 Bytes = 17179869176/1024/1024/1024 ≈ 16GB, "or about 16GB" should not 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 pull request #19025: [SPARK-21813][core] Modify TaskMemoryManager.MAXI...

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

    https://github.com/apache/spark/pull/19025#discussion_r134657758
  
    --- Diff: core/src/main/java/org/apache/spark/memory/TaskMemoryManager.java ---
    @@ -53,7 +53,7 @@
      * retrieve the base object.
      * <p>
      * This allows us to address 8192 pages. In on-heap mode, the maximum page size is limited by the
    - * maximum size of a long[] array, allowing us to address 8192 * 2^32 * 8 bytes, which is
    + * maximum size of a long[] array, allowing us to address 8192 * 2^31 * 8 bytes, which is
    --- End diff --
    
    This is still wrong -- 2^31 - 1.
    The amount of memory is more like 140TB, not 35.


---
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 #19025: [SPARK-21813][core] Modify TaskMemoryManager.MAXI...

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

    https://github.com/apache/spark/pull/19025#discussion_r135200981
  
    --- Diff: core/src/main/java/org/apache/spark/memory/TaskMemoryManager.java ---
    @@ -53,7 +53,7 @@
      * retrieve the base object.
      * <p>
      * This allows us to address 8192 pages. In on-heap mode, the maximum page size is limited by the
    - * maximum size of a long[] array, allowing us to address 8192 * 2^32 * 8 bytes, which is
    + * maximum size of a long[] array, allowing us to address 8192 * 2^31 * 8 bytes, which is
    --- End diff --
    
    Oh, I see. Thanks for the answer


---
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 #19025: [SPARK-21813][core] Modify TaskMemoryManager.MAXIMUM_PAG...

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

    https://github.com/apache/spark/pull/19025
  
    Merged to master (warning above is spurious)


---
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 #19025: [SPARK-21813][core] Modify TaskMemoryManager.MAXI...

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

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


---
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 #19025: [SPARK-21813][core] Modify TaskMemoryManager.MAXIMUM_PAG...

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

    https://github.com/apache/spark/pull/19025
  
    Can one of the admins verify this patch?


---
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 #19025: [SPARK-21813][core] Modify TaskMemoryManager.MAXIMUM_PAG...

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

    https://github.com/apache/spark/pull/19025
  
    **[Test build #3909 has started](https://amplab.cs.berkeley.edu/jenkins/job/NewSparkPullRequestBuilder/3909/testReport)** for PR 19025 at commit [`06f80ad`](https://github.com/apache/spark/commit/06f80ad0d30b9a420632efc53d8143e3580eb6ef).


---
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 #19025: [SPARK-21813][core] Modify TaskMemoryManager.MAXI...

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

    https://github.com/apache/spark/pull/19025#discussion_r135754290
  
    --- Diff: core/src/main/java/org/apache/spark/memory/TaskMemoryManager.java ---
    @@ -74,7 +74,7 @@
        * Maximum supported data page size (in bytes). In principle, the maximum addressable page size is
        * (1L &lt;&lt; OFFSET_BITS) bytes, which is 2+ petabytes. However, the on-heap allocator's
        * maximum page size is limited by the maximum amount of data that can be stored in a long[]
    -   * array, which is (2^32 - 1) * 8 bytes (or 16 gigabytes). Therefore, we cap this at 16 gigabytes.
    +   * array, which is (2^31 - 1) * 8 bytes (or 17 gigabytes). Therefore, we cap this at 17 gigabytes.
    --- End diff --
    
    ok,I have modified 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 #19025: [SPARK-21813][core] Modify TaskMemoryManager.MAXI...

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

    https://github.com/apache/spark/pull/19025#discussion_r135195514
  
    --- Diff: core/src/main/java/org/apache/spark/memory/TaskMemoryManager.java ---
    @@ -53,7 +53,7 @@
      * retrieve the base object.
      * <p>
      * This allows us to address 8192 pages. In on-heap mode, the maximum page size is limited by the
    - * maximum size of a long[] array, allowing us to address 8192 * 2^32 * 8 bytes, which is
    + * maximum size of a long[] array, allowing us to address 8192 * 2^31 * 8 bytes, which is
    --- End diff --
    
    8192*(2^31-1)*8 = 1.4073749e+14. A terabyte is 10^12 bytes, so that's about 140. I think you're thinking that giga/terabytes are a power of 2 bytes, and they're not. 
    
    The 35 error comes from omitting the factor of 8 (8x too small), but having an array size 2x too long (2^32). It should net-net be 4x larger. 35 * 4 = 140


---
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 #19025: [SPARK-21813][core] Modify TaskMemoryManager.MAXI...

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

    https://github.com/apache/spark/pull/19025#discussion_r134657843
  
    --- Diff: core/src/main/java/org/apache/spark/memory/TaskMemoryManager.java ---
    @@ -74,7 +74,7 @@
        * Maximum supported data page size (in bytes). In principle, the maximum addressable page size is
        * (1L &lt;&lt; OFFSET_BITS) bytes, which is 2+ petabytes. However, the on-heap allocator's
        * maximum page size is limited by the maximum amount of data that can be stored in a long[]
    -   * array, which is (2^32 - 1) * 8 bytes (or 16 gigabytes). Therefore, we cap this at 16 gigabytes.
    +   * array, which is (2^31 - 1) * 8 bytes (or 16 gigabytes). Therefore, we cap this at 16 gigabytes.
    --- End diff --
    
    "or about 17 gigabytes" is correct, and it's not capped at 16 gigabytes, but the value in the previous sentence. I'd just say "cap at this value".


---
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 #19025: [SPARK-21813][core] Modify TaskMemoryManager.MAXIMUM_PAG...

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

    https://github.com/apache/spark/pull/19025
  
    **[Test build #3909 has finished](https://amplab.cs.berkeley.edu/jenkins/job/NewSparkPullRequestBuilder/3909/testReport)** for PR 19025 at commit [`06f80ad`](https://github.com/apache/spark/commit/06f80ad0d30b9a420632efc53d8143e3580eb6ef).
     * This patch passes all tests.
     * This patch **does not merge 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 #19025: [SPARK-21813][core] Modify TaskMemoryManager.MAXI...

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

    https://github.com/apache/spark/pull/19025#discussion_r135383867
  
    --- Diff: core/src/main/java/org/apache/spark/memory/TaskMemoryManager.java ---
    @@ -74,7 +74,7 @@
        * Maximum supported data page size (in bytes). In principle, the maximum addressable page size is
        * (1L &lt;&lt; OFFSET_BITS) bytes, which is 2+ petabytes. However, the on-heap allocator's
        * maximum page size is limited by the maximum amount of data that can be stored in a long[]
    -   * array, which is (2^32 - 1) * 8 bytes (or 16 gigabytes). Therefore, we cap this at 16 gigabytes.
    +   * array, which is (2^31 - 1) * 8 bytes (or 17 gigabytes). Therefore, we cap this at 17 gigabytes.
    --- End diff --
    
    Or *about* 17 gigabytes


---
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