Skip to content

Add benchmark for quadratic hash performance #7617

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

Merged
merged 2 commits into from
Feb 28, 2017

Conversation

natecook1000
Copy link
Member

This is a hopefully reasonably quick benchmark for the issue described in SR-3268. There's a fix in ca99ee2c342e07a277938ba2bb1b5bfd418ce0c2 that can be tested once the benchmark is in.

(This includes an update to the CMakeList template, since the autogenerated file was edited in a previous commit.)

@natecook1000 natecook1000 requested a review from Gankra February 19, 2017 17:54
@lplarson
Copy link
Contributor

@natecook1000 Your CMakeLists.txt_template changes aren't in the actual CMakeLists.txt file. Run the scripts/generate_harness/generate_harness.py script to generate it from the template.

@natecook1000
Copy link
Member Author

The problem was that someone made changes to the generated version and checked that in — my changes to the template only reflect that change.

@lplarson
Copy link
Contributor

@swift-ci benchmark

@swift-ci
Copy link
Contributor

Build comment file:

Optimized (O)


@lplarson
Copy link
Contributor

  • /Users/buildnode/jenkins/workspace/swift-PR-osx-perf/Ninja-Release/swift-macosx-x86_64/bin/Benchmark_Driver compare --log-dir /Users/buildnode/jenkins/workspace/swift-PR-osx-perf/Ninja-Release/swift-macosx-x86_64/benchmark/logs --swift-repo /Users/buildnode/jenkins/workspace/swift-PR-osx-perf/swift --compare-script /Users/buildnode/jenkins/workspace/swift-PR-osx-perf/swift/benchmark/scripts/compare_perf_tests.py
    Traceback (most recent call last):
    File "/Users/buildnode/jenkins/workspace/swift-PR-osx-perf/swift/benchmark/scripts/compare_perf_tests.py", line 371, in
    sys.exit(main())
    File "/Users/buildnode/jenkins/workspace/swift-PR-osx-perf/swift/benchmark/scripts/compare_perf_tests.py", line 154, in main
    ratio = (old_results[key] + 0.001) / (new_results[key] + 0.001)
    KeyError: 'MapReduceAnyCollectionShort'
    Traceback (most recent call last):
    File "/Users/buildnode/jenkins/workspace/swift-PR-osx-perf/swift/benchmark/scripts/compare_perf_tests.py", line 371, in
    sys.exit(main())
    File "/Users/buildnode/jenkins/workspace/swift-PR-osx-perf/swift/benchmark/scripts/compare_perf_tests.py", line 154, in main
    ratio = (old_results[key] + 0.001) / (new_results[key] + 0.001)
    KeyError: 'MapReduceAnyCollectionShort'
    branch/branch comparison skipped: no previous branch logs
    Comparing master/Benchmark_O-20170227132347.log HEAD/Benchmark_O-20170227144206.log ...
    Comparing master/Benchmark_Onone-20170227133644.log HEAD/Benchmark_Onone-20170227145919.log ...
    -- check-swift-benchmark-macosx-x86_64 finished --

@natecook1000
Copy link
Member Author

@lplarson I don't think it knows how to calculate the benchmark comparison when there's a new one added; had the same problem with #6382.

@natecook1000
Copy link
Member Author

@swift-ci Please smoke test

@lplarson
Copy link
Contributor

You're right. LGTM.

@natecook1000 natecook1000 merged commit 252af7d into swiftlang:master Feb 28, 2017
dict2[k] = v
}

CheckResults(dict2[size/2] == dict2[size/2],
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@natecook1000 My apologies for missing this. It appears you're trying to compare dict1 and dict2 but you have dict2 on both sides.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're so right! I think that will still defeat inlining, but I'll fix ASAP. Thanks!

@natecook1000 natecook1000 deleted the nc-hash-quadtest branch March 4, 2017 10:23
@palimondo
Copy link
Contributor

palimondo commented May 16, 2017

@natecook1000:

This is a hopefully reasonably quick benchmark

On my Late 2008 MBP, it takes 40s to run 1 iteration in O build and 640s in Onone build! For 20 iterations, my machine spends more than 3,5 hours on this single performance test. Could we adjust the size to something more reasonable? It should take less than a second to run one iteration of test run, the benchmark driver will up the N in the outer loop as necessary to run for about a second.

// cc @lplarson @dabrahams

@palimondo
Copy link
Contributor

The CI machine is about 10x faster than my MBP, but it still takes 6s in O, and 50s in Onone build per iteration. That is overall about 20 minutes on this single test.

@natecook1000
Copy link
Member Author

The idea was to actually fix this issue in short order so the test wouldn't actually be quadratic. We definitely don't need to be slowing down the entire benchmarking suite if we don't have a fix in flight.

@palimondo
Copy link
Contributor

Umm... you've referenced fix in commit ca99ee2, did that not land? I'm confused about where that one commit comes from...

@palimondo
Copy link
Contributor

It seems it is in the tree... did that not fix the issue?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants