Let's also add a build-time option, possibly enabled in production builds someday, to always zero memory before returning it.

Additionally, maybe zero on free.

Additionally, do some smartness to avoid double-zeroing (a free immediately followed by an allocation).

Doing this in production would all be gated on not exploding perf, obviously.

> Doing this in production would all be gated on not exploding perf, obviously.

I'd be a bit surprised if this didn't happen. The last time I touched the allocator shim the perf waterfall shouted at me because I just introduced by accident an extra mfence on the allocator path ( Issue 593344 ). Here you are talking about doing a memset per allocation (well, free), hmm. May the force be with you !

What are these microbenchmarks which are so sensitive to the allocator path?

I just added memset to the allocator shim and run a few random benchmarks (blink_perf, octane etc.) on Linux desktop and Android and results are inconclusive, some benchmarks show improvement, some show regression (either I am running them wrong or they naturally vary that much). It would be nice to know what is the reliable and predictable Chrome benchmark set for our purposes.

Well, the 'top level' speed UMA metrics are go/cr-speed-rel-metrics but I see those are deprecated... :shrug:

mfence could be infinitely more expensive than a small memzero.
alekseys@ attempted a local experiment and the results didn't show much difference.
It would be lovely to conduct *trustworthy* performance measurements. 

> What are these microbenchmarks which are so sensitive to the allocator path?
Take a look to https://chromeperf.appspot.com/group_report?rev=380377
that's what shouted at me back then with my http://crrev.com/380377.

Unfortunately virtually any micro benchmark would be sensible to this, and there are hundreds on the waterfall. Pragmatically speaking IMHO trying all of them, on all supported HW combinations, is going to be unfeasible. I'd probably consider just going for it and see what the perf waterfall says. You don't need to wait that a perf sheriff picks up the alert, you can see all the alerts that cover a given revision using
https://chromeperf.appspot.com/group_report?rev=380377, where 380377 is the Cr-Commit-Position number of the CL.

> mfence could be infinitely more expensive than a small memzero.
Thinking more on this, true.

Good luck.

maybe +dtu, +sullivan can give more pointers on how to try this on the new pinpoint service for try jobs. I don't think there is support yet to try on "all HW combinations", but that might come some time soon?

The feature request for "all HW combinations" is https://github.com/catapult-project/catapult/issues/4209 -- Dave, is that still planned for this week?

It's slated for next week (3/19-3/23)

For those interested:

https://chromium-review.googlesource.com/c/chromium/src/+/1134523
https://pinpoint-dot-chromeperf.appspot.com/job/1423dc34a40000

We'll see!

The real Pinpoint results (speedometer2) are here: https://pinpoint-dot-chromeperf.appspot.com/job/14f7545ea40000

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d5d8bc3a2b11f043e08bf56b6a724e7e960ef07f

commit d5d8bc3a2b11f043e08bf56b6a724e7e960ef07f
Author: Chris Palmer <palmer@chromium.org>
Date: Thu Aug 02 20:18:12 2018

[PartitionAlloc] Poison memory regions when freeing.

A last-ditch sanity check in case of memory corruption.

Bug:  680657 
Change-Id: I79934eceb4fcb6ce64422cee5004f057d40a6cab
Reviewed-on: https://chromium-review.googlesource.com/1134523
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Commit-Queue: Chris Palmer <palmer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#580297}
[modify] https://crrev.com/d5d8bc3a2b11f043e08bf56b6a724e7e960ef07f/base/allocator/partition_allocator/partition_alloc.h

I don't think there's anything specific left to be done on this bug; if we think of anything, we can start a new bug for it.

There is also issue 714341, however.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/96acdb1abb37f8e49b35fa2bdcfda7a93cee4ab6

commit 96acdb1abb37f8e49b35fa2bdcfda7a93cee4ab6
Author: Chris Palmer <palmer@chromium.org>
Date: Tue Aug 07 00:35:25 2018

Ensure that `PartitionAllocGenericFlags` callers are only using valid flags.

Bug:  680657 
Change-Id: I2d879f5b28c392a083fbd867a3eb05568d19dc67
Reviewed-on: https://chromium-review.googlesource.com/1164252
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Commit-Queue: Chris Palmer <palmer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#581049}
[modify] https://crrev.com/96acdb1abb37f8e49b35fa2bdcfda7a93cee4ab6/base/allocator/partition_allocator/partition_alloc.h
[modify] https://crrev.com/96acdb1abb37f8e49b35fa2bdcfda7a93cee4ab6/base/allocator/partition_allocator/partition_alloc_constants.h

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b60e5cc0bd4b351c5898dc3637f537c355546c11

commit b60e5cc0bd4b351c5898dc3637f537c355546c11
Author: Chris Palmer <palmer@chromium.org>
Date: Fri Sep 07 00:51:36 2018

[PartitionAlloc] Add a flag to allow callers to zero-fill allocations.

This enables callers to avoid doing it themselves (centralizing the `memset` in
1 place, rather than duplicating it at every call site that wants it), and to
allow the PartitionAlloc implementation to avoid doing it itself if the
operating system has already zero'd the new page(s).

Bug:  864462 , 680657 
Change-Id: Ibeae0bf7b9f20238334bf9bc022d0f243a77ef11
Reviewed-on: https://chromium-review.googlesource.com/1162825
Commit-Queue: Chris Palmer <palmer@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Albert J. Wong <ajwong@chromium.org>
Cr-Commit-Position: refs/heads/master@{#589389}
[modify] https://crrev.com/b60e5cc0bd4b351c5898dc3637f537c355546c11/base/allocator/partition_allocator/partition_alloc.cc
[modify] https://crrev.com/b60e5cc0bd4b351c5898dc3637f537c355546c11/base/allocator/partition_allocator/partition_alloc.h
[modify] https://crrev.com/b60e5cc0bd4b351c5898dc3637f537c355546c11/base/allocator/partition_allocator/partition_alloc_constants.h
[modify] https://crrev.com/b60e5cc0bd4b351c5898dc3637f537c355546c11/base/allocator/partition_allocator/partition_alloc_unittest.cc
[modify] https://crrev.com/b60e5cc0bd4b351c5898dc3637f537c355546c11/base/allocator/partition_allocator/partition_bucket.cc
[modify] https://crrev.com/b60e5cc0bd4b351c5898dc3637f537c355546c11/base/allocator/partition_allocator/partition_cookie.h
[modify] https://crrev.com/b60e5cc0bd4b351c5898dc3637f537c355546c11/base/allocator/partition_allocator/partition_page.h

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f94b670562073c228efaf1f32a20708dc0425db3

commit f94b670562073c228efaf1f32a20708dc0425db3
Author: Findit <findit-for-me@appspot.gserviceaccount.com>
Date: Fri Sep 07 01:37:54 2018

Revert "[PartitionAlloc] Add a flag to allow callers to zero-fill allocations."

This reverts commit b60e5cc0bd4b351c5898dc3637f537c355546c11.

Reason for revert:

Findit (https://goo.gl/kROfz5) identified CL at revision 589389 as the
culprit for failures in the build cycles as shown on:
https://findit-for-me.appspot.com/waterfall/culprit?key=ag9zfmZpbmRpdC1mb3ItbWVyRAsSDVdmU3VzcGVjdGVkQ0wiMWNocm9taXVtL2I2MGU1Y2MwYmQ0YjM1MWM1ODk4ZGMzNjM3ZjUzN2MzNTU1NDZjMTEM

Sample Failed Build: https://ci.chromium.org/buildbot/chromium.linux/Cast%20Audio%20Linux/20069

Sample Failed Step: base_unittests

Original change's description:
> [PartitionAlloc] Add a flag to allow callers to zero-fill allocations.
> 
> This enables callers to avoid doing it themselves (centralizing the `memset` in
> 1 place, rather than duplicating it at every call site that wants it), and to
> allow the PartitionAlloc implementation to avoid doing it itself if the
> operating system has already zero'd the new page(s).
> 
> Bug:  864462 , 680657 
> Change-Id: Ibeae0bf7b9f20238334bf9bc022d0f243a77ef11
> Reviewed-on: https://chromium-review.googlesource.com/1162825
> Commit-Queue: Chris Palmer <palmer@chromium.org>
> Reviewed-by: Kentaro Hara <haraken@chromium.org>
> Reviewed-by: Albert J. Wong <ajwong@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#589389}

Change-Id: Ifd54700a2acb141c458df69304d7ccdce2719cc5
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  864462 , 680657 
Reviewed-on: https://chromium-review.googlesource.com/1212346
Cr-Commit-Position: refs/heads/master@{#589404}
[modify] https://crrev.com/f94b670562073c228efaf1f32a20708dc0425db3/base/allocator/partition_allocator/partition_alloc.cc
[modify] https://crrev.com/f94b670562073c228efaf1f32a20708dc0425db3/base/allocator/partition_allocator/partition_alloc.h
[modify] https://crrev.com/f94b670562073c228efaf1f32a20708dc0425db3/base/allocator/partition_allocator/partition_alloc_constants.h
[modify] https://crrev.com/f94b670562073c228efaf1f32a20708dc0425db3/base/allocator/partition_allocator/partition_alloc_unittest.cc
[modify] https://crrev.com/f94b670562073c228efaf1f32a20708dc0425db3/base/allocator/partition_allocator/partition_bucket.cc
[modify] https://crrev.com/f94b670562073c228efaf1f32a20708dc0425db3/base/allocator/partition_allocator/partition_cookie.h
[modify] https://crrev.com/f94b670562073c228efaf1f32a20708dc0425db3/base/allocator/partition_allocator/partition_page.h

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/7bd9d693667d585c44d2d29666860c678c19bc83

commit 7bd9d693667d585c44d2d29666860c678c19bc83
Author: Chris Palmer <palmer@chromium.org>
Date: Fri Sep 07 23:27:10 2018

[PartitionAlloc] Add a flag to allow callers to zero-fill allocations.

This enables callers to avoid doing it themselves (centralizing the `memset` in
1 place, rather than duplicating it at every call site that wants it), and to
allow the PartitionAlloc implementation to avoid doing it itself if the
operating system has already zero'd the new page(s).

This is a re-land, with fixes, of
https://chromium-review.googlesource.com/c/chromium/src/+/1162825.

Bug:  864462 , 680657 
TBR: ajwong,haraken
Change-Id: Icfe05bca82c79c8a8029db40b2902271d3d10347
Reviewed-on: https://chromium-review.googlesource.com/1214173
Reviewed-by: Chris Palmer <palmer@chromium.org>
Commit-Queue: Chris Palmer <palmer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#589717}
[modify] https://crrev.com/7bd9d693667d585c44d2d29666860c678c19bc83/base/allocator/partition_allocator/partition_alloc.cc
[modify] https://crrev.com/7bd9d693667d585c44d2d29666860c678c19bc83/base/allocator/partition_allocator/partition_alloc.h
[modify] https://crrev.com/7bd9d693667d585c44d2d29666860c678c19bc83/base/allocator/partition_allocator/partition_alloc_constants.h
[modify] https://crrev.com/7bd9d693667d585c44d2d29666860c678c19bc83/base/allocator/partition_allocator/partition_alloc_unittest.cc
[modify] https://crrev.com/7bd9d693667d585c44d2d29666860c678c19bc83/base/allocator/partition_allocator/partition_bucket.cc
[modify] https://crrev.com/7bd9d693667d585c44d2d29666860c678c19bc83/base/allocator/partition_allocator/partition_cookie.h
[modify] https://crrev.com/7bd9d693667d585c44d2d29666860c678c19bc83/base/allocator/partition_allocator/partition_page.h

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/99391f020f67be766ae8203c4563510b19916494

commit 99391f020f67be766ae8203c4563510b19916494
Author: Chris Palmer <palmer@chromium.org>
Date: Tue Sep 11 22:38:05 2018

[PartitionAlloc] Don't poison memory on free.

It might? be the cause of a perf regression.

Bug:  882471 , 680657 
Change-Id: Iec34a348426ccfdf5864a3b2cbe0c377da20ac0e
Reviewed-on: https://chromium-review.googlesource.com/1220468
Commit-Queue: Chris Palmer <palmer@chromium.org>
Reviewed-by: Emil A Eklund <eae@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#590508}
[modify] https://crrev.com/99391f020f67be766ae8203c4563510b19916494/base/allocator/partition_allocator/partition_page.h

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/7b006ff1e72e73263ef4e462a1e7324327e864ee

commit 7b006ff1e72e73263ef4e462a1e7324327e864ee
Author: Chris Palmer <palmer@chromium.org>
Date: Wed Sep 12 00:18:56 2018

Revert "[PartitionAlloc] Don't poison memory on free."

This reverts commit 99391f020f67be766ae8203c4563510b19916494.

Reason for revert: <INSERT REASONING HERE>

Original change's description:
> [PartitionAlloc] Don't poison memory on free.
> 
> It might? be the cause of a perf regression.
> 
> Bug:  882471 , 680657 
> Change-Id: Iec34a348426ccfdf5864a3b2cbe0c377da20ac0e
> Reviewed-on: https://chromium-review.googlesource.com/1220468
> Commit-Queue: Chris Palmer <palmer@chromium.org>
> Reviewed-by: Emil A Eklund <eae@chromium.org>
> Reviewed-by: Kentaro Hara <haraken@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#590508}

TBR=palmer@chromium.org,eae@chromium.org,haraken@chromium.org

Change-Id: I6c687b8cb2b8aa0bc3de682c1c39f430b1f6b05c
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  882471 ,  680657 
Reviewed-on: https://chromium-review.googlesource.com/1220815
Reviewed-by: Chris Palmer <palmer@chromium.org>
Commit-Queue: Chris Palmer <palmer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#590545}
[modify] https://crrev.com/7b006ff1e72e73263ef4e462a1e7324327e864ee/base/allocator/partition_allocator/partition_page.h

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a027d75bf6661b126d95a5032ea45c6ad2a6d0a9

commit a027d75bf6661b126d95a5032ea45c6ad2a6d0a9
Author: Chris Palmer <palmer@chromium.org>
Date: Wed Sep 12 20:15:26 2018

[PartitionAlloc] Don't poison memory on free.

It seems to have been the cause of a perf regression.

This is a re-land of
https://chromium-review.googlesource.com/c/chromium/src/+/1220468, with a fixed
test: the code and unit tests need to have matching behavior and expectations
when `DCHECK_IS_ON()`.

Bug:  882471 , 680657 , 883144 
Change-Id: I7afb7c8d9f57431dfd4e264a6a22711660e040d6
Reviewed-on: https://chromium-review.googlesource.com/1222290
Reviewed-by: Emil A Eklund <eae@chromium.org>
Commit-Queue: Emil A Eklund <eae@chromium.org>
Cr-Commit-Position: refs/heads/master@{#590800}
[modify] https://crrev.com/a027d75bf6661b126d95a5032ea45c6ad2a6d0a9/base/allocator/partition_allocator/partition_alloc_unittest.cc
[modify] https://crrev.com/a027d75bf6661b126d95a5032ea45c6ad2a6d0a9/base/allocator/partition_allocator/partition_page.h