In the process of debugging https://bugs.webkit.org/show_bug.cgi?id=154091 I added new capabilities to MallocBench. These include: - Added a recording of http://nim-lang.org/docs/lib.html. - Added thread id to the recording and the ability to playback switching threads in MallocBench - Added aligned allocations to recordings and the ability to playback - Added --use-thread-id option to honor recorded thread ids - Added --detailed-report to output remaining allocations by size after playback - Added --no-warmup to not run the warm up iteration
Created attachment 271832 [details] Patch I did not include the new nimlang.ops in this patch. I'll land it separately.
Attachment 271832 [details] did not pass style-queue: ERROR: PerformanceTests/MallocBench/MallocBench/stress.h:31: benchmark_stress is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: PerformanceTests/MallocBench/MallocBench/theverge.cpp:45: benchmark_theverge is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: PerformanceTests/MallocBench/MallocBench/theverge.cpp:57: benchmark_theverge_memory_warning is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: PerformanceTests/MallocBench/MallocBench/theverge.h:31: benchmark_theverge is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: PerformanceTests/MallocBench/MallocBench/theverge.h:32: benchmark_theverge_memory_warning is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: PerformanceTests/MallocBench/MallocBench/alloc_free.cpp:26: Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted. [build/include_order] [4] ERROR: PerformanceTests/MallocBench/MallocBench/alloc_free.cpp:31: benchmark_alloc_free is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: PerformanceTests/MallocBench/MallocBench/big.cpp:42: benchmark_big is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: PerformanceTests/MallocBench/MallocBench/tree.cpp:178: benchmark_tree_allocate is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: PerformanceTests/MallocBench/MallocBench/tree.cpp:193: benchmark_tree_traverse is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: PerformanceTests/MallocBench/MallocBench/tree.cpp:208: benchmark_tree_churn is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: PerformanceTests/MallocBench/MallocBench/flickr.h:31: benchmark_flickr is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: PerformanceTests/MallocBench/MallocBench/flickr.h:32: benchmark_flickr_memory_warning is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: PerformanceTests/MallocBench/MallocBench/Interpreter.cpp:33: Streams are highly discouraged. [readability/streams] [3] ERROR: PerformanceTests/MallocBench/MallocBench/Interpreter.cpp:35: Alphabetical sorting problem. [build/include_order] [4] ERROR: PerformanceTests/MallocBench/MallocBench/Interpreter.cpp:160: Multi line control clauses should use braces. [whitespace/braces] [4] ERROR: PerformanceTests/MallocBench/MallocBench/Interpreter.cpp:161: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: PerformanceTests/MallocBench/MallocBench/Interpreter.cpp:166: Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons. [readability/comparison_to_zero] [5] ERROR: PerformanceTests/MallocBench/MallocBench/Interpreter.cpp:173: Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons. [readability/comparison_to_zero] [5] ERROR: PerformanceTests/MallocBench/MallocBench/Interpreter.cpp:175: More than one command on the same line [whitespace/newline] [4] ERROR: PerformanceTests/MallocBench/MallocBench/Interpreter.cpp:175: Missing space before { [whitespace/braces] [5] ERROR: PerformanceTests/MallocBench/MallocBench/Interpreter.cpp:175: Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons. [readability/comparison_to_zero] [5] ERROR: PerformanceTests/MallocBench/MallocBench/Interpreter.cpp:246: A case label should not be indented, but line up with its switch statement. [whitespace/indent] [4] ERROR: PerformanceTests/MallocBench/MallocBench/Interpreter.cpp:307: More than one command on the same line [whitespace/newline] [4] ERROR: PerformanceTests/MallocBench/MallocBench/Interpreter.cpp:307: Missing space before { [whitespace/braces] [5] ERROR: PerformanceTests/MallocBench/MallocBench/Interpreter.cpp:315: Could not find a newline character at the end of the file. [whitespace/ending_newline] [5] ERROR: PerformanceTests/MallocBench/MallocBench/tree.h:31: benchmark_tree_allocate is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: PerformanceTests/MallocBench/MallocBench/tree.h:32: benchmark_tree_traverse is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: PerformanceTests/MallocBench/MallocBench/tree.h:33: benchmark_tree_churn is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: PerformanceTests/MallocBench/MallocBench/medium.cpp:42: benchmark_medium is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: PerformanceTests/MallocBench/MallocBench/flickr.cpp:45: benchmark_flickr is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: PerformanceTests/MallocBench/MallocBench/flickr.cpp:57: benchmark_flickr_memory_warning is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: PerformanceTests/MallocBench/MallocBench/message.cpp:113: benchmark_message_one is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: PerformanceTests/MallocBench/MallocBench/message.cpp:141: benchmark_message_many is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: PerformanceTests/MallocBench/MallocBench/reddit.h:31: benchmark_reddit is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: PerformanceTests/MallocBench/MallocBench/reddit.h:32: benchmark_reddit_memory_warning is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: PerformanceTests/MallocBench/MallocBench/nimlang.h:31: benchmark_nimlang is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: PerformanceTests/MallocBench/MallocBench/medium.h:31: benchmark_medium is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: PerformanceTests/MallocBench/MallocBench/nimlang.cpp:26: Found other header before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted. [build/include_order] [4] ERROR: PerformanceTests/MallocBench/MallocBench/nimlang.cpp:28: Found header this file implements after other header. Should be: config.h, primary header, blank line, and then alphabetically sorted. [build/include_order] [4] ERROR: PerformanceTests/MallocBench/MallocBench/nimlang.cpp:31: "cstddef" already included at PerformanceTests/MallocBench/MallocBench/nimlang.cpp:30 [build/include] [4] ERROR: PerformanceTests/MallocBench/MallocBench/nimlang.cpp:43: Bad include order. Mixing system and custom headers. [build/include_order] [4] ERROR: PerformanceTests/MallocBench/MallocBench/nimlang.cpp:45: benchmark_nimlang is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: PerformanceTests/MallocBench/MallocBench/list.cpp:99: benchmark_list_allocate is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: PerformanceTests/MallocBench/MallocBench/list.cpp:127: benchmark_list_traverse is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: PerformanceTests/MallocBench/MallocBench/alloc_free.h:31: benchmark_alloc_free is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: PerformanceTests/MallocBench/MallocBench/list.h:31: benchmark_list_allocate is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: PerformanceTests/MallocBench/MallocBench/list.h:32: benchmark_list_traverse is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: PerformanceTests/MallocBench/MallocBench/fragment.h:31: benchmark_fragment is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: PerformanceTests/MallocBench/MallocBench/fragment.h:32: benchmark_fragment_iterate is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: PerformanceTests/MallocBench/MallocBench/stress_aligned.h:31: benchmark_stress_aligned is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: PerformanceTests/MallocBench/MallocBench/fragment.cpp:82: benchmark_fragment is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: PerformanceTests/MallocBench/MallocBench/fragment.cpp:110: benchmark_fragment_iterate is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: PerformanceTests/MallocBench/MallocBench/big.h:31: benchmark_big is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: PerformanceTests/MallocBench/MallocBench/balloon.h:31: benchmark_balloon is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: PerformanceTests/MallocBench/MallocBench/balloon.cpp:36: benchmark_balloon is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: PerformanceTests/MallocBench/MallocBench/facebook.cpp:45: benchmark_facebook is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: PerformanceTests/MallocBench/MallocBench/memalign.cpp:30: Bad include order. Mixing system and custom headers. [build/include_order] [4] ERROR: PerformanceTests/MallocBench/MallocBench/memalign.cpp:44: benchmark_memalign is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: PerformanceTests/MallocBench/MallocBench/realloc.h:31: benchmark_realloc is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: PerformanceTests/MallocBench/MallocBench/realloc.cpp:32: benchmark_realloc is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: PerformanceTests/MallocBench/MallocBench/memalign.h:31: benchmark_memalign is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: PerformanceTests/MallocBench/MallocBench/facebook.h:31: benchmark_facebook is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: PerformanceTests/MallocBench/MallocBench/Interpreter.h:45: enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums. [readability/enum_casing] [4] ERROR: PerformanceTests/MallocBench/MallocBench/Interpreter.h:46: More than one command on the same line [whitespace/newline] [4] ERROR: PerformanceTests/MallocBench/MallocBench/Interpreter.h:50: This { should be at the end of the previous line [whitespace/braces] [4] ERROR: PerformanceTests/MallocBench/MallocBench/Interpreter.h:61: Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons. [readability/comparison_to_zero] [5] ERROR: PerformanceTests/MallocBench/MallocBench/stress.cpp:123: benchmark_stress is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: PerformanceTests/MallocBench/MallocBench/message.h:31: benchmark_message_one is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: PerformanceTests/MallocBench/MallocBench/message.h:32: benchmark_message_many is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: PerformanceTests/MallocBench/MallocBench/churn.h:31: benchmark_churn is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: PerformanceTests/MallocBench/MallocBench/churn.cpp:42: benchmark_churn is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: PerformanceTests/MallocBench/MallocBench/reddit.cpp:45: benchmark_reddit is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: PerformanceTests/MallocBench/MallocBench/reddit.cpp:57: benchmark_reddit_memory_warning is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: PerformanceTests/MallocBench/MallocBench/stress_aligned.cpp:140: benchmark_stress_aligned is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: PerformanceTests/MallocBench/MallocBench/CommandLine.h:60: Could not find a newline character at the end of the file. [whitespace/ending_newline] [5] Total errors found: 76 in 45 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 271832 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=271832&action=review r=me > PerformanceTests/MallocBench/MallocBench/Interpreter.cpp:148 > + if (m_useThreadId && (runThreadId != threadId)) { > + switchToThread(threadId); > + break; > + } This won't be very useful for testing throughput since it will artificially incur synchronization overhead. But I guess it could be useful to catching bugs.
(In reply to comment #3) > Comment on attachment 271832 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=271832&action=review > > r=me > > > PerformanceTests/MallocBench/MallocBench/Interpreter.cpp:148 > > + if (m_useThreadId && (runThreadId != threadId)) { > > + switchToThread(threadId); > > + break; > > + } > > This won't be very useful for testing throughput since it will artificially > incur synchronization overhead. But I guess it could be useful to catching > bugs. Agreed.
Comment on attachment 271832 [details] Patch Rejecting attachment 271832 [details] from commit-queue. Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-02', 'land-attachment', '--force-clean', '--non-interactive', '--parent-command=commit-queue', 271832, '--port=mac']" exit_code: 2 cwd: /Volumes/Data/EWS/WebKit Last 500 characters of output: /git.webkit.org/WebKit abd2e00..3c44510 master -> origin/master Partial-rebuilding .git/svn/refs/remotes/origin/master/.rev_map.268f45cc-cd09-0410-ab3c-d52691b4dbfc ... Currently at 196951 = abd2e004968543c90d575b9444879cfd024dae2f r196952 = 3c44510dcb10a0de38505ef976c12b1e83cac895 Done rebuilding .git/svn/refs/remotes/origin/master/.rev_map.268f45cc-cd09-0410-ab3c-d52691b4dbfc First, rewinding head to replay your work on top of it... Fast-forwarded master to refs/remotes/origin/master. Full output: http://webkit-queues.webkit.org/results/869131
Committed r196955: <http://trac.webkit.org/changeset/196955>
Comment on attachment 271832 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=271832&action=review > PerformanceTests/MallocBench/MallocBench/Interpreter.cpp:97 > + fprintf(stderr, "Done running\n"); Revert this too -- it clutters the output and seems accidental. If it's valuable, you can conditionalize it based on --detailed-report. > PerformanceTests/MallocBench/MallocBench/Interpreter.cpp:241 > +static void writeData(void* start, size_t size) > +{ > + char* writePtr = reinterpret_cast<char*>(start); > + > + for (size_t sizeLeft = size; !!sizeLeft; ) { > + size_t sizeThisIter = std::min(sizeLeft, 4096ul); > + > + writePtr[0] = random() & 0xff; > + writePtr[1] = random() & 0xff; > + writePtr[2] = random() & 0xff; > + writePtr[3] = random() & 0xff; > + > + writePtr += sizeThisIter; > + sizeLeft -= sizeThisIter; > + } > +} I didn't notice this change initially. I think you should revert it. I'm not familiar with any programs that write to four random byte locations inside each 4kB of memory. Most programs I'm familiar with initialize (all of) their allocated memory, and then read it a bunch too. The one exception I can think of is vector-like objects, which may initialize only 50%. The performance of a memory allocator depends not only on the speed of its function calls but also on the speed of *reading and writing* the memory it returns. If our benchmark neglects to read or write 99.9% of the memory it allocates, it misses an important aspect of performance. Our old behavior used zeroing to reflect the bare minimum memory traffic that we expect each allocation to experience. (Most allocations will experience more, since they will read after writing.) There may be an argument for doing more reads and writes -- but I don't think there can be an argument for doing fewer.
(In reply to comment #7) > Comment on attachment 271832 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=271832&action=review > > > PerformanceTests/MallocBench/MallocBench/Interpreter.cpp:97 > > + fprintf(stderr, "Done running\n"); > > Revert this too -- it clutters the output and seems accidental. If it's > valuable, you can conditionalize it based on --detailed-report. > > > PerformanceTests/MallocBench/MallocBench/Interpreter.cpp:241 > > +static void writeData(void* start, size_t size) > > +{ > > + char* writePtr = reinterpret_cast<char*>(start); > > + > > + for (size_t sizeLeft = size; !!sizeLeft; ) { > > + size_t sizeThisIter = std::min(sizeLeft, 4096ul); > > + > > + writePtr[0] = random() & 0xff; > > + writePtr[1] = random() & 0xff; > > + writePtr[2] = random() & 0xff; > > + writePtr[3] = random() & 0xff; > > + > > + writePtr += sizeThisIter; > > + sizeLeft -= sizeThisIter; > > + } > > +} > > I didn't notice this change initially. I think you should revert it. > > I'm not familiar with any programs that write to four random byte locations > inside each 4kB of memory. Most programs I'm familiar with initialize (all > of) their allocated memory, and then read it a bunch too. The one exception > I can think of is vector-like objects, which may initialize only 50%. > > The performance of a memory allocator depends not only on the speed of its > function calls but also on the speed of *reading and writing* the memory it > returns. If our benchmark neglects to read or write 99.9% of the memory it > allocates, it misses an important aspect of performance. > > Our old behavior used zeroing to reflect the bare minimum memory traffic > that we expect each allocation to experience. (Most allocations will > experience more, since they will read after writing.) There may be an > argument for doing more reads and writes -- but I don't think there can be > an argument for doing fewer. I'll revert both of these.