hadoop-common-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Colin Patrick McCabe (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (HADOOP-11660) Add support for hardware crc on ARM aarch64 architecture
Date Thu, 26 Mar 2015 21:34:55 GMT

    [ https://issues.apache.org/jira/browse/HADOOP-11660?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14382721#comment-14382721
] 

Colin Patrick McCabe commented on HADOOP-11660:
-----------------------------------------------

Thank you for your patience, [~enevill].  I think this is almost ready to commit.

{code}
168	ELSEIF (CMAKE_SYSTEM_PROCESSOR STREQUAL "aarch64")
169	  set(BULK_CRC_ARCH_SOURCE_FIlE "${D}/util/bulk_crc32_aarch64.c")
170	ENDIF()
{code}
Can you put in a {{MESSAGE}} here that explains that the architecture is unsupported in the
ELSE case?  We certainly don't want to be losing hardware acceleration without being aware
of it.

{{bulk_crc32_aarch64.c}}: you should include {{stdint.h}} here for {{uint8_t}}, etc.  Even
though some other header is probably pulling it in now by accident.

{{bulk_crc32_x86.c}}: I would really prefer not to wrap this in a giant {{#if defined(__GNUC__)
&& !defined(__FreeBSD__)}}, especially since we're not wrapping the ARM version like
that.  If people want this to be compiler and os-specific, it would be better to do it at
the CMake level.  I would say just take that out and let people fix it if it becomes a problem
for them.

Can you post before / after performance numbers for x86_64?  Maybe you can instrument test_bulk_crc32.c
to produce those numbers.

It looks like when this was done previously, the test code was not checked in.  See:
https://issues.apache.org/jira/browse/HADOOP-7446?focusedCommentId=13084519&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-13084519

I'm sorry to dump another task on you, but my co-workers will kill me if I regress checksum
performance.

Thanks again for working on this.  As soon as we verify that we haven't regressed perf, and
made those minor changes, we should be good to go.

> Add support for hardware crc on ARM aarch64 architecture
> --------------------------------------------------------
>
>                 Key: HADOOP-11660
>                 URL: https://issues.apache.org/jira/browse/HADOOP-11660
>             Project: Hadoop Common
>          Issue Type: Improvement
>          Components: native
>    Affects Versions: 3.0.0
>         Environment: ARM aarch64 development platform
>            Reporter: Edward Nevill
>            Assignee: Edward Nevill
>            Priority: Minor
>              Labels: performance
>         Attachments: jira-11660.patch
>
>   Original Estimate: 48h
>  Remaining Estimate: 48h
>
> This patch adds support for hardware crc for ARM's new 64 bit architecture
> The patch is completely conditionalized on __aarch64__
> I have only added support for the non pipelined version as I benchmarked the pipelined
version on aarch64 and it showed no performance improvement.
> The aarch64 version supports both Castagnoli and Zlib CRCs as both of these are supported
on ARM aarch64 hardwre.
> To benchmark this I modified the test_bulk_crc32 test to print out the time taken to
CRC a 1MB dataset 1000 times.
> Before:
> CRC 1048576 bytes @ 512 bytes per checksum X 1000 iterations = 2.55
> CRC 1048576 bytes @ 512 bytes per checksum X 1000 iterations = 2.55
> After:
> CRC 1048576 bytes @ 512 bytes per checksum X 1000 iterations = 0.57
> CRC 1048576 bytes @ 512 bytes per checksum X 1000 iterations = 0.57
> So this represents a 5X performance improvement on raw CRC calculation.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Mime
View raw message