Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Alternative solution to the unaligned accesses. #7708

Merged
merged 2 commits into from Apr 15, 2019

Conversation

Projects
None yet
4 participants
@omoerbeek
Copy link
Member

commented Apr 11, 2019

No tricks with the alignment of the union, just do a explicit memcpy.
#7688

Checklist

I have:

  • read the CONTRIBUTING.md document
  • compiled this code
  • tested this code
  • included documentation (including possible behaviour changes)
  • documented the code
  • added or modified regression test(s)
  • added or modified unit test(s)
Alternative solution to the unaligned accesses. No tricks with the al…
…ignment of the union,

just do a explicit memcpy.

@omoerbeek omoerbeek requested a review from Habbie Apr 11, 2019

@omoerbeek

This comment has been minimized.

Copy link
Member Author

commented Apr 11, 2019

I the meantime tested on OpenBSD/amd64, Linux/amd64 (via Travis), OpenBSD/arm and Linux/arm (debian stretch). OpenBSD/sparc64 still building.

@rgacogne

This comment has been minimized.

Copy link
Member

commented Apr 11, 2019

I don't really expect a meaningful difference but can we quickly check the performance impact of this PR? dnsdist's dynamic blocks performance is critical and is mostly tied to these of the NetmaskTree.

@Habbie

This comment has been minimized.

Copy link
Member

commented Apr 11, 2019

On a real Pi:

(summary: good)

peter@lorentz:~/pdns/pdns/recursordist $ ./testrunner --run_test=syncres_cc/test_throttled_server_count
Running 1 test case...

*** No errors detected
peter@lorentz:~/pdns/pdns/recursordist $ valgrind ./testrunner --run_test=syncres_cc/test_throttled_server_count
==4176== Memcheck, a memory error detector
==4176== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==4176== Using Valgrind-3.13.0 and LibVEX; rerun with -h for copyright info
==4176== Command: ./testrunner --run_test=syncres_cc/test_throttled_server_count
==4176==
==4176== Invalid read of size 8
==4176==    at 0x4865C90: ??? (in /usr/lib/arm-linux-gnueabihf/libarmmem.so)
==4176==  Address 0x51fdc28 is 16 bytes inside a block of size 23 alloc'd
==4176==    at 0x4847D4C: operator new(unsigned int) (vg_replace_malloc.c:328)
==4176==    by 0x48F53E3: ??? (in /usr/lib/arm-linux-gnueabihf/libboost_unit_test_framework.so.1.62.0)
==4176==    by 0x48BE9FF: ??? (in /usr/lib/arm-linux-gnueabihf/libboost_unit_test_framework.so.1.62.0)
==4176==    by 0x48BED37: ??? (in /usr/lib/arm-linux-gnueabihf/libboost_unit_test_framework.so.1.62.0)
==4176==
Running 1 test case...

*** No errors detected
==4176==
==4176== HEAP SUMMARY:
==4176==     in use at exit: 8,293 bytes in 209 blocks
==4176==   total heap usage: 7,896 allocs, 7,687 frees, 654,244 bytes allocated
==4176==
==4176== LEAK SUMMARY:
==4176==    definitely lost: 0 bytes in 0 blocks
==4176==    indirectly lost: 0 bytes in 0 blocks
==4176==      possibly lost: 0 bytes in 0 blocks
==4176==    still reachable: 8,293 bytes in 209 blocks
==4176==         suppressed: 0 bytes in 0 blocks
==4176== Rerun with --leak-check=full to see details of leaked memory
==4176==
==4176== For counts of detected and suppressed errors, rerun with: -v
==4176== ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 6 from 3)
@Habbie

Habbie approved these changes Apr 11, 2019

@omoerbeek

This comment has been minimized.

Copy link
Member Author

commented Apr 15, 2019

Speed test show sno significant difference. I assume the most important factor is the scanning for the bits in the bitset. So I'll merge this.

@omoerbeek omoerbeek merged commit e71722a into PowerDNS:master Apr 15, 2019

0 of 2 checks passed

ci/circleci: build CircleCI is running your tests
Details
continuous-integration/travis-ci/pr The Travis CI build is in progress
Details

@omoerbeek omoerbeek deleted the omoerbeek:rec-7688-alternative-fix branch Apr 15, 2019

@omoerbeek omoerbeek referenced this pull request Apr 15, 2019

Closed

Work around debian stretch on arm problem. #7707

3 of 7 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.