[jira] [Commented] (HDFS-6560) Byte array native checksumming on DN side

2014-07-15 Thread Todd Lipcon (JIRA)

[ 
https://issues.apache.org/jira/browse/HDFS-6560?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14062561#comment-14062561
 ] 

Todd Lipcon commented on HDFS-6560:
---

A few notes on the patch:

{code}
+  sums_addr = (*env)-GetPrimitiveArrayCritical(env, j_sums, NULL);
+  data_addr = (*env)-GetPrimitiveArrayCritical(env, j_data, NULL);
+
+  if (unlikely(!sums_addr || !data_addr)) {
+(*env)-ReleasePrimitiveArrayCritical(env, j_data, data_addr, 0);
+(*env)-ReleasePrimitiveArrayCritical(env, j_sums, sums_addr, 0);
{code}

Here it seems like you might call Release() on a NULL address. I can't tell 
from reading the spec whether that's safe or not, but maybe best to guard the 
Release calls and only release non-NULL addresses.

{code}
+  ret = bulk_verify_crc(data, MIN(numChecksumsInMB * bytes_per_checksum,
+data_len - checksumNum * bytes_per_checksum), sums,
+crc_type, bytes_per_checksum, error_data);
{code}
style nit: given that the second line here is an argument to MIN, it should 
probably wrap more like:
{code}
  ret = bulk_verify_crc(data, MIN(numChecksumsInMB * bytes_per_checksum,
  data_len - checksumNum * 
bytes_per_checksum),
sums, crc_type, bytes_per_checksum, error_data);
{code}

or assign the MIN result to a temporary value like 'len'

{code}
+long pos = base_pos + (error_data.bad_data - data) + checksumNum *
+bytes_per_checksum;
{code}
style: indentation is off a bit here (continuation line should indent)

Also, I'll move this to the HADOOP project since it only affects code in common/

 Byte array native checksumming on DN side
 -

 Key: HDFS-6560
 URL: https://issues.apache.org/jira/browse/HDFS-6560
 Project: Hadoop HDFS
  Issue Type: Sub-task
  Components: performance
Reporter: James Thomas
Assignee: James Thomas
 Attachments: HDFS-3528.patch, HDFS-6560.patch






--
This message was sent by Atlassian JIRA
(v6.2#6252)


[jira] [Commented] (HDFS-6560) Byte array native checksumming on DN side

2014-07-11 Thread Todd Lipcon (JIRA)

[ 
https://issues.apache.org/jira/browse/HDFS-6560?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14058997#comment-14058997
 ] 

Todd Lipcon commented on HDFS-6560:
---

Nice benchmark results. Worth noting that your results here also include the 
startup/shutdown time of the DNs and client, right? In that case, the 10% 
reduction in cycles is an under-estimate. You could quantify this by getting 
the perf results for a put of a 1GB file, as well as the results for a 2GB 
file, and subtracting. That would give you a better indication of the savings 
in the steady state (once all JIT has kicked in, etc).

Nicholas -- for the write path, wouldn't we nearly always be writing with 
CRC32C? The only case we'd be writing with the old checksum format is distcping 
from Hadoop 1.0 clusters or appending to files which have existed since Hadoop 
1.0. Both are far less common. So, this change is a positive change in that it 
would increase the performance a lot for the common case (and maybe only 
decrease a couple percent for the uncommon case). The comparative results for 
CRC32 (zlib polynomial) seem a bit orthogonal to this JIRA.

 Byte array native checksumming on DN side
 -

 Key: HDFS-6560
 URL: https://issues.apache.org/jira/browse/HDFS-6560
 Project: Hadoop HDFS
  Issue Type: Sub-task
  Components: datanode, hdfs-client, performance
Reporter: James Thomas
Assignee: James Thomas
 Attachments: HDFS-3528.patch, HDFS-6560.patch






--
This message was sent by Atlassian JIRA
(v6.2#6252)


[jira] [Commented] (HDFS-6560) Byte array native checksumming on DN side

2014-07-11 Thread Tsz Wo Nicholas Sze (JIRA)

[ 
https://issues.apache.org/jira/browse/HDFS-6560?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14059035#comment-14059035
 ] 

Tsz Wo Nicholas Sze commented on HDFS-6560:
---

For CRC32C, I have no problem to use native.  But, for CRC32, I don't see a 
point to have our native implementation since it is unlikely to be better than 
zip.  Also, these two algorithms could coexist, i.e. use our native 
implementation for CRC32C and use zip for CRC32.  No?

 Byte array native checksumming on DN side
 -

 Key: HDFS-6560
 URL: https://issues.apache.org/jira/browse/HDFS-6560
 Project: Hadoop HDFS
  Issue Type: Sub-task
  Components: datanode, hdfs-client, performance
Reporter: James Thomas
Assignee: James Thomas
 Attachments: HDFS-3528.patch, HDFS-6560.patch






--
This message was sent by Atlassian JIRA
(v6.2#6252)


[jira] [Commented] (HDFS-6560) Byte array native checksumming on DN side

2014-07-11 Thread Todd Lipcon (JIRA)

[ 
https://issues.apache.org/jira/browse/HDFS-6560?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14059358#comment-14059358
 ] 

Todd Lipcon commented on HDFS-6560:
---

Sure, makes sense. I just think that discussion belongs in your JIRA about 
smartly switching between native and Java checksums, whereas this JIRA 
(enabling native checksum more consistently on both read and write paths) is a 
separate problem to solve. Your point of using zip CRC32 makes sense on both 
the read and write paths.

 Byte array native checksumming on DN side
 -

 Key: HDFS-6560
 URL: https://issues.apache.org/jira/browse/HDFS-6560
 Project: Hadoop HDFS
  Issue Type: Sub-task
  Components: datanode, hdfs-client, performance
Reporter: James Thomas
Assignee: James Thomas
 Attachments: HDFS-3528.patch, HDFS-6560.patch






--
This message was sent by Atlassian JIRA
(v6.2#6252)


[jira] [Commented] (HDFS-6560) Byte array native checksumming on DN side

2014-07-11 Thread Todd Lipcon (JIRA)

[ 
https://issues.apache.org/jira/browse/HDFS-6560?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14059362#comment-14059362
 ] 

Todd Lipcon commented on HDFS-6560:
---

BTW, I'd add that, per your findings, native is always faster when 
bytesPerChecksum = 512. I've never seen an HDFS deployment set 
bytesPerChecksum to any value other than 512, so maybe it's a moot point?

 Byte array native checksumming on DN side
 -

 Key: HDFS-6560
 URL: https://issues.apache.org/jira/browse/HDFS-6560
 Project: Hadoop HDFS
  Issue Type: Sub-task
  Components: datanode, hdfs-client, performance
Reporter: James Thomas
Assignee: James Thomas
 Attachments: HDFS-3528.patch, HDFS-6560.patch






--
This message was sent by Atlassian JIRA
(v6.2#6252)


[jira] [Commented] (HDFS-6560) Byte array native checksumming on DN side

2014-07-11 Thread Tsz Wo Nicholas Sze (JIRA)

[ 
https://issues.apache.org/jira/browse/HDFS-6560?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14059392#comment-14059392
 ] 

Tsz Wo Nicholas Sze commented on HDFS-6560:
---

Hi Todd, for bytesPerChecksum == 512, native is not strictly better than zip as 
shown in [this 
table|https://issues.apache.org/jira/browse/HDFS-6560?focusedCommentId=14044060page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14044060].
  Also, my test ran on direct buffer case which our native implementation seems 
to have some advantages.  Without direct buffer, I guess our native 
implementation may be even worse.

BTW, do you want to review HADOOP-10778?  Or I could find someone else.

 Byte array native checksumming on DN side
 -

 Key: HDFS-6560
 URL: https://issues.apache.org/jira/browse/HDFS-6560
 Project: Hadoop HDFS
  Issue Type: Sub-task
  Components: datanode, hdfs-client, performance
Reporter: James Thomas
Assignee: James Thomas
 Attachments: HDFS-3528.patch, HDFS-6560.patch






--
This message was sent by Atlassian JIRA
(v6.2#6252)


[jira] [Commented] (HDFS-6560) Byte array native checksumming on DN side

2014-07-11 Thread Todd Lipcon (JIRA)

[ 
https://issues.apache.org/jira/browse/HDFS-6560?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14059413#comment-14059413
 ] 

Todd Lipcon commented on HDFS-6560:
---

I think using GetPrimitiveArrayCritical, our byte-buffer implementation should 
be pretty comparable to the direct-buffer one. Based on my experience 
elsewhere, GetPrimitiveArrayCritical is pretty fast - only a few cycles to 
check a per-thread variable, and we only pay that cost once per MB.

James's benchmarks in the comment higher up this JIRA also seem to confirm this 
- the direct buffer impl and the byte[] impl are within a couple percent (maybe 
just experimental error at that point).

I'm not sure I'll have a chance to review HADOOP-10778 in detail - the code to 
switch between native and java checksumming looks fine, but would require a bit 
more time to look in detail at the new loop structure and the benchmark. If 
you're in a rush to get it committed, probably best to find someone else to 
review it. (I'm traveling this week and next on pseudo vacation)

 Byte array native checksumming on DN side
 -

 Key: HDFS-6560
 URL: https://issues.apache.org/jira/browse/HDFS-6560
 Project: Hadoop HDFS
  Issue Type: Sub-task
  Components: datanode, hdfs-client, performance
Reporter: James Thomas
Assignee: James Thomas
 Attachments: HDFS-3528.patch, HDFS-6560.patch






--
This message was sent by Atlassian JIRA
(v6.2#6252)


[jira] [Commented] (HDFS-6560) Byte array native checksumming on DN side

2014-07-10 Thread Tsz Wo Nicholas Sze (JIRA)

[ 
https://issues.apache.org/jira/browse/HDFS-6560?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14058232#comment-14058232
 ] 

Tsz Wo Nicholas Sze commented on HDFS-6560:
---

[~james.thomas], I suspect our native implementation for CRC32 may not be 
faster than zip.CRC32.  Could you run some benchmarks?  You may use the 
performance test posted in HADOOP-10778.

 Byte array native checksumming on DN side
 -

 Key: HDFS-6560
 URL: https://issues.apache.org/jira/browse/HDFS-6560
 Project: Hadoop HDFS
  Issue Type: Sub-task
  Components: datanode, hdfs-client, performance
Reporter: James Thomas
Assignee: James Thomas
 Attachments: HDFS-3528.patch, HDFS-6560.patch






--
This message was sent by Atlassian JIRA
(v6.2#6252)


[jira] [Commented] (HDFS-6560) Byte array native checksumming on DN side

2014-07-10 Thread Hadoop QA (JIRA)

[ 
https://issues.apache.org/jira/browse/HDFS-6560?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14058238#comment-14058238
 ] 

Hadoop QA commented on HDFS-6560:
-

{color:red}-1 overall{color}.  Here are the results of testing the latest 
attachment 
  http://issues.apache.org/jira/secure/attachment/12655126/HDFS-6560.patch
  against trunk revision .

{color:green}+1 @author{color}.  The patch does not contain any @author 
tags.

{color:red}-1 tests included{color}.  The patch doesn't appear to include 
any new or modified tests.
Please justify why no new tests are needed for this 
patch.
Also please list what manual steps were performed to 
verify this patch.

{color:green}+1 javac{color}.  The applied patch does not increase the 
total number of javac compiler warnings.

{color:green}+1 javadoc{color}.  There were no new javadoc warning messages.

{color:green}+1 eclipse:eclipse{color}.  The patch built with 
eclipse:eclipse.

{color:green}+1 findbugs{color}.  The patch does not introduce any new 
Findbugs (version 2.0.3) warnings.

{color:green}+1 release audit{color}.  The applied patch does not increase 
the total number of release audit warnings.

{color:red}-1 core tests{color}.  The patch failed these unit tests in 
hadoop-common-project/hadoop-common:

  org.apache.hadoop.fs.TestDFVariations
  org.apache.hadoop.fs.TestSymlinkLocalFSFileSystem
  org.apache.hadoop.ipc.TestIPC
  org.apache.hadoop.fs.TestSymlinkLocalFSFileContext

{color:green}+1 contrib tests{color}.  The patch passed contrib unit tests.

Test results: 
https://builds.apache.org/job/PreCommit-HDFS-Build/7327//testReport/
Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/7327//console

This message is automatically generated.

 Byte array native checksumming on DN side
 -

 Key: HDFS-6560
 URL: https://issues.apache.org/jira/browse/HDFS-6560
 Project: Hadoop HDFS
  Issue Type: Sub-task
  Components: datanode, hdfs-client, performance
Reporter: James Thomas
Assignee: James Thomas
 Attachments: HDFS-3528.patch, HDFS-6560.patch






--
This message was sent by Atlassian JIRA
(v6.2#6252)


[jira] [Commented] (HDFS-6560) Byte array native checksumming on DN side

2014-07-03 Thread Todd Lipcon (JIRA)

[ 
https://issues.apache.org/jira/browse/HDFS-6560?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14052043#comment-14052043
 ] 

Todd Lipcon commented on HDFS-6560:
---

Nicholas -- your results are cool that Java 7 improved the CRC32 performance. 
But aren't most clusters now running with CRC32C enabled by default, and the 
native implementation for Crc32C remains significantly faster than our Java 
one? So, this JIRA still would be very good for most people's performance.

 Byte array native checksumming on DN side
 -

 Key: HDFS-6560
 URL: https://issues.apache.org/jira/browse/HDFS-6560
 Project: Hadoop HDFS
  Issue Type: Sub-task
  Components: datanode, hdfs-client, performance
Reporter: James Thomas
Assignee: Todd Lipcon
 Attachments: HDFS-3528.patch






--
This message was sent by Atlassian JIRA
(v6.2#6252)


[jira] [Commented] (HDFS-6560) Byte array native checksumming on DN side

2014-07-03 Thread Tsz Wo Nicholas Sze (JIRA)

[ 
https://issues.apache.org/jira/browse/HDFS-6560?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14052104#comment-14052104
 ] 

Tsz Wo Nicholas Sze commented on HDFS-6560:
---

Sure. I just want to point out that we should not blindly use native for all 
cases.  In the current implementation, native will be used if it is available.  
However, native is not always the best according to the benchmarks.

 Byte array native checksumming on DN side
 -

 Key: HDFS-6560
 URL: https://issues.apache.org/jira/browse/HDFS-6560
 Project: Hadoop HDFS
  Issue Type: Sub-task
  Components: datanode, hdfs-client, performance
Reporter: James Thomas
Assignee: Todd Lipcon
 Attachments: HDFS-3528.patch






--
This message was sent by Atlassian JIRA
(v6.2#6252)


[jira] [Commented] (HDFS-6560) Byte array native checksumming on DN side

2014-06-25 Thread Tsz Wo Nicholas Sze (JIRA)

[ 
https://issues.apache.org/jira/browse/HDFS-6560?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14044060#comment-14044060
 ] 

Tsz Wo Nicholas Sze commented on HDFS-6560:
---

For Java 7, NativeCrc32 with direct buffer is faster than zip.CRC32 for 
byte-per-crc  512 but slower than it for byte-per-crc  512.  For byte-per-crc 
== 512 (which is an important case), their performances are similar.

{noformat}
 java.version = 1.7.0_60
java.runtime.name = Java(TM) SE Runtime Environment
 java.runtime.version = 1.7.0_60-b19
  java.vm.version = 24.60-b09
   java.vm.vendor = Oracle Corporation
 java.vm.name = Java HotSpot(TM) 64-Bit Server VM
java.vm.specification.version = 1.7
   java.specification.version = 1.7
  os.arch = x86_64
  os.name = Mac OS X
   os.version = 10.9.3
DATA_LENGTH = 67108864
TRIALS  = 10
{noformat}
Performance Table (bpc is byte-per-crc in MB/sec; #T = #Theads)
|  bpc  | #T ||  Zip || PureJava | % diff ||   Native | % diff | % diff |
|32 |  1 | 237.7 | 789.8 | 232.3% |1624.2 | 583.3% | 105.7% |
|32 |  2 | 207.6 | 604.5 | 191.2% |1608.3 | 674.8% | 166.1% |
|32 |  4 | 179.8 | 609.8 | 239.2% |1387.8 | 671.9% | 127.6% |
|32 |  8 | 163.4 | 356.8 | 118.3% | 910.4 | 457.1% | 155.1% |
|32 | 16 |  81.6 | 183.7 | 125.0% | 490.9 | 501.4% | 167.3% |
|  bpc  | #T ||  Zip || PureJava | % diff ||   Native | % diff | % diff |
|64 |  1 | 423.7 |1027.0 | 142.4% |1654.4 | 290.4% |  61.1% |
|64 |  2 | 417.7 |1031.8 | 147.0% |1640.1 | 292.7% |  59.0% |
|64 |  4 | 366.0 | 693.8 |  89.5% |1381.7 | 277.5% |  99.2% |
|64 |  8 | 280.2 | 443.5 |  58.3% |1046.8 | 273.5% | 136.0% |
|64 | 16 | 143.3 | 233.0 |  62.6% | 556.3 | 288.2% | 138.8% |
|  bpc  | #T ||  Zip || PureJava | % diff ||   Native | % diff | % diff |
|   128 |  1 | 716.1 |1229.9 |  71.7% |1628.6 | 127.4% |  32.4% |
|   128 |  2 | 703.0 |1221.4 |  73.7% |1610.0 | 129.0% |  31.8% |
|   128 |  4 | 708.1 | 998.7 |  41.0% |1408.1 |  98.8% |  41.0% |
|   128 |  8 | 503.3 | 583.7 |  16.0% |1059.4 | 110.5% |  81.5% |
|   128 | 16 | 259.6 | 316.4 |  21.9% | 610.3 | 135.2% |  92.9% |
|  bpc  | #T ||  Zip || PureJava | % diff ||   Native | % diff | % diff |
|   256 |  1 |1217.4 |1346.6 |  10.6% |1554.3 |  27.7% |  15.4% |
|   256 |  2 |1186.3 |1339.0 |  12.9% |1556.6 |  31.2% |  16.3% |
|   256 |  4 |1094.9 |1102.9 |   0.7% |1389.3 |  26.9% |  26.0% |
|   256 |  8 | 768.3 | 656.8 | -14.5% |1109.4 |  44.4% |  68.9% |
|   256 | 16 | 394.6 | 358.7 |  -9.1% | 597.8 |  51.5% |  66.7% |
|  bpc  | #T ||  Zip || PureJava | % diff ||   Native | % diff | % diff |
|   512 |  1 |1632.0 |1391.3 | -14.7% |1548.1 |  -5.1% |  11.3% |
|   512 |  2 |1608.9 |1377.8 | -14.4% |1550.1 |  -3.7% |  12.5% |
|   512 |  4 |1465.2 |1092.6 | -25.4% |1420.8 |  -3.0% |  30.0% |
|   512 |  8 |1027.7 | 721.7 | -29.8% |1124.4 |   9.4% |  55.8% |
|   512 | 16 | 551.6 | 397.9 | -27.9% | 628.2 |  13.9% |  57.9% |
|  bpc  | #T ||  Zip || PureJava | % diff ||   Native | % diff | % diff |
|  1024 |  1 |1980.3 |1411.7 | -28.7% |1570.7 | -20.7% |  11.3% |
|  1024 |  2 |1909.4 |1396.7 | -26.9% |1534.7 | -19.6% |   9.9% |
|  1024 |  4 |1747.4 |1159.9 | -33.6% |1426.2 | -18.4% |  23.0% |
|  1024 |  8 |1245.6 | 752.7 | -39.6% |1149.8 |  -7.7% |  52.8% |
|  1024 | 16 | 660.6 | 380.2 | -42.4% | 618.1 |  -6.4% |  62.6% |
|  bpc  | #T ||  Zip || PureJava | % diff ||   Native | % diff | % diff |
|  2048 |  1 |2140.4 |1390.2 | -35.0% |1570.3 | -26.6% |  13.0% |
|  2048 |  2 |2126.5 |1374.5 | -35.4% |1538.9 | -27.6% |  12.0% |
|  2048 |  4 |1769.0 |1132.9 | -36.0% |1411.5 | -20.2% |  24.6% |
|  2048 |  8 |1358.6 | 754.8 | -44.4% |1207.0 | -11.2% |  59.9% |
|  2048 | 16 | 749.4 | 394.4 | -47.4% | 639.9 | -14.6% |  62.2% |
|  bpc  | #T ||  Zip || PureJava | % diff ||   Native | % diff | % diff |
|  4096 |  1 |2325.5 |1427.0 | -38.6% |1531.2 | -34.2% |   7.3% |
|  4096 |  2 |2199.7 |1375.1 | -37.5% |1524.4 | -30.7% |  10.9% |
|  4096 |  4 |1927.3 |1103.8 | -42.7% |1412.7 | -26.7% |  28.0% |
|  4096 |  8 |1427.1 | 773.2 | -45.8% |1206.2 | -15.5% |  56.0% |
|  4096 | 16 | 761.0 | 401.3 | -47.3% | 632.6 | -16.9% |  57.6% |
|  bpc  | #T ||  Zip || PureJava | % diff ||   Native | % diff | % diff |
|  8192 |  1 |2364.7 |1431.6 | -39.5% |1566.2 | -33.8% 

[jira] [Commented] (HDFS-6560) Byte array native checksumming on DN side

2014-06-24 Thread Tsz Wo Nicholas Sze (JIRA)

[ 
https://issues.apache.org/jira/browse/HDFS-6560?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14042491#comment-14042491
 ] 

Tsz Wo Nicholas Sze commented on HDFS-6560:
---

By the Java implementation, do you mean PureJavaCrc32?  Which JDK are you using?

For Java 6, my patch in HADOOP-10674 will improve 40% - 90% performance.  For 
Java 7 or 8, it turns out that the performance of java.util.zip.CRC32 is very 
good.  So, it becomes tricky to select the best implementation across 
platforms.  We should add a performance test similar to 
TestPureJavaCrc32.PerformanceTest for testing the native crc implementations.

 Byte array native checksumming on DN side
 -

 Key: HDFS-6560
 URL: https://issues.apache.org/jira/browse/HDFS-6560
 Project: Hadoop HDFS
  Issue Type: Sub-task
  Components: datanode, hdfs-client, performance
Reporter: James Thomas
Assignee: Todd Lipcon
 Attachments: HDFS-3528.patch






--
This message was sent by Atlassian JIRA
(v6.2#6252)


[jira] [Commented] (HDFS-6560) Byte array native checksumming on DN side

2014-06-24 Thread James Thomas (JIRA)

[ 
https://issues.apache.org/jira/browse/HDFS-6560?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14042609#comment-14042609
 ] 

James Thomas commented on HDFS-6560:


I am using JDK 1.7. And yes, I mean the verifyChunkedSums code path in 
DataChecksum that calls out repeatedly to either PureJavaCrc32 or 
PureJavaCrc32C.

Do you think the improvements you made to the Java code could also be made to 
the native implementation?

 Byte array native checksumming on DN side
 -

 Key: HDFS-6560
 URL: https://issues.apache.org/jira/browse/HDFS-6560
 Project: Hadoop HDFS
  Issue Type: Sub-task
  Components: datanode, hdfs-client, performance
Reporter: James Thomas
Assignee: Todd Lipcon
 Attachments: HDFS-3528.patch






--
This message was sent by Atlassian JIRA
(v6.2#6252)


[jira] [Commented] (HDFS-6560) Byte array native checksumming on DN side

2014-06-24 Thread Tsz Wo Nicholas Sze (JIRA)

[ 
https://issues.apache.org/jira/browse/HDFS-6560?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14042674#comment-14042674
 ] 

Tsz Wo Nicholas Sze commented on HDFS-6560:
---

We should be able to improve current native implementation.  For example, the 
*, / and % operators is currently used but the can be easily avoided.  I also 
have some other ideas but need to test them.

 Byte array native checksumming on DN side
 -

 Key: HDFS-6560
 URL: https://issues.apache.org/jira/browse/HDFS-6560
 Project: Hadoop HDFS
  Issue Type: Sub-task
  Components: datanode, hdfs-client, performance
Reporter: James Thomas
Assignee: Todd Lipcon
 Attachments: HDFS-3528.patch






--
This message was sent by Atlassian JIRA
(v6.2#6252)


[jira] [Commented] (HDFS-6560) Byte array native checksumming on DN side

2014-06-23 Thread Colin Patrick McCabe (JIRA)

[ 
https://issues.apache.org/jira/browse/HDFS-6560?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14041477#comment-14041477
 ] 

Colin Patrick McCabe commented on HDFS-6560:


{code}
+  public static void verifyChunkedSumsByteArray(int bytesPerSum,
+  int checksumType, byte[] sums, int sumsOffset, byte[] data,
+  int dataOffset, int dataLength, String fileName, long basePos)
+  throws ChecksumException {
+nativeVerifyChunkedSumsByteArray(bytesPerSum, checksumType,
+sums, sumsOffset,
+data, dataOffset, dataLength,
+fileName, basePos);
+  }
{code}
What's the purpose of this wrapper function?  It just passes all its arguments 
directly to the other function. Public functions can have the native annotation 
too.

{code}
+  sums_addr = (*env)-GetPrimitiveArrayCritical(env, j_sums, NULL);
+  data_addr = (*env)-GetPrimitiveArrayCritical(env, j_data, NULL);
+
+  if (unlikely(!sums_addr || !data_addr)) {
+THROW(env, java/lang/OutOfMemoryError,
+  not enough memory for byte arrays in JNI code);
+return;
+  }
{code}

This is going to leak memory if {{GetPrimitiveArrayCritical}} succeeds for 
{{sums_addr}} but not for {{data_addr}}.

 Byte array native checksumming on DN side
 -

 Key: HDFS-6560
 URL: https://issues.apache.org/jira/browse/HDFS-6560
 Project: Hadoop HDFS
  Issue Type: Sub-task
  Components: datanode, hdfs-client, performance
Reporter: James Thomas
Assignee: James Thomas
 Attachments: HDFS-3528.patch






--
This message was sent by Atlassian JIRA
(v6.2#6252)


[jira] [Commented] (HDFS-6560) Byte array native checksumming on DN side

2014-06-23 Thread Todd Lipcon (JIRA)

[ 
https://issues.apache.org/jira/browse/HDFS-6560?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14041647#comment-14041647
 ] 

Todd Lipcon commented on HDFS-6560:
---

In addition to being a memory leak, it's illegal to THROW when holding the 
critical section.

I agree that you won't see an improvement in wall clock write speed if you're 
disk bound. However, if you run perf stat on the datanode and upload a file, 
you should see a big reduction in number of CPU-seconds used. That would 
translate to more CPU left for other work on the cluster, and therefore better 
overall throughput. Any chance you've given that a try?

Some back of the envelope math: given your results above on the 100MB buffer, 
it seems like we should expect about 67ms of CPU time saved per 100MB. So, on a 
terasort (which writes three replicas and thus 3TB of data) we would expect 
around 2000 cpu-seconds saved across the cluster. On a small 10 node x 8 core 
cluster, this ought to save us about on the order of 25 seconds of wall clock, 
which is pretty good! If we can make the same improvement in the MR checksum 
code (calculate and verify) and the DFS client calculation, that's another 3TB 
(and thus 2000 cpu seconds) worth of savings. I expect actual wall clock 
savings to be a little less substantial since not all phases are CPU bound, but 
I bet we do see a noticeable improvement once all this work is done.

 Byte array native checksumming on DN side
 -

 Key: HDFS-6560
 URL: https://issues.apache.org/jira/browse/HDFS-6560
 Project: Hadoop HDFS
  Issue Type: Sub-task
  Components: datanode, hdfs-client, performance
Reporter: James Thomas
Assignee: Todd Lipcon
 Attachments: HDFS-3528.patch






--
This message was sent by Atlassian JIRA
(v6.2#6252)


[jira] [Commented] (HDFS-6560) Byte array native checksumming on DN side

2014-06-23 Thread Todd Lipcon (JIRA)

[ 
https://issues.apache.org/jira/browse/HDFS-6560?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14041649#comment-14041649
 ] 

Todd Lipcon commented on HDFS-6560:
---

Actually had a dumb moment there with the back-of-the-envelope: terasort output 
is only written with replication 1, so it won't see quite the benefits I was 
hoping for. However, with the MR shuffle CRC32C and the DFS client/DN CRC32C, 
we should see savings of 4TB worth of checksum, which is still 2680 cpu seconds 
overall.

 Byte array native checksumming on DN side
 -

 Key: HDFS-6560
 URL: https://issues.apache.org/jira/browse/HDFS-6560
 Project: Hadoop HDFS
  Issue Type: Sub-task
  Components: datanode, hdfs-client, performance
Reporter: James Thomas
Assignee: Todd Lipcon
 Attachments: HDFS-3528.patch






--
This message was sent by Atlassian JIRA
(v6.2#6252)