I think we should be willing to take a temporary performance hit on the dev
channel in the interests of generating data which will eventually improve
stability.
Could we set jemalloc on selected renderer processes?  I realize that
wouldn't necessarily only impact the target domains, but it might be better
than making the change global.

-scott


On Thu, Oct 1, 2009 at 10:47 AM, Jim Roskind <[email protected]> wrote:

> I'm quite sure it is jemalloc.  I failed miserably by not
> over-communicating this checkin.  My apologies to all that I have
> inconvenience.
> We have just this week identified a number of misteps in our use of
> TCMalloc.  I think that we can eventually reach an excellent result with
> TCMalloc... and we had a few fixes... but we also wanted to be able to talk
> more precisely about how things contrast with jemalloc (as folks ask us why
> we're looking so intently at tcmalloc).
>
> If folks are cool with it, it might be interesting to push the dev build
> with this in place.  JEMalloc appears to be very conservative in its memory
> usage, but doesn't get the speed we've seen with TCMalloc.  We can already
> see some specific areas where the current (checked in) incarnation of
> TCMalloc stumbles (example: today, TCMalloc never decommits memory in the
> browser process... yeah... that's just a bug... but I was wondering how much
> it hurt... and a switch to jemalloc would instantly provide a temporary
> fix).   It may be interesting to see the response on the dev channel to this
> temporary change.
>
> Side benefits include some feedback on crashers, as each allocator
> illuminates a different set of corruption problems ("if" we have any :-) ).
>
> Do other folks agree it would be interesting to see a dev build go out with
> this in place??
>
> Thanks,
>
> Jim
>
>
> On Thu, Oct 1, 2009 at 8:46 AM, Mike Belshe <[email protected]> wrote:
>
>> Probably caused by jemalloc.  Jim was experimenting with the idea of
>> getting some dev-channel data from using jemalloc as opposed to tcmalloc.
>>  I'm not surprised there was a perf delta, but I am surprised by how much.
>>  The DOM benchmark in this test dropped by 8%.  Several other benchmarks
>> took similar hits.
>> http://build.chromium.org/buildbot/perf/vista-release-dual-core/dom_perf/report.html?history=150
>>
>> http://build.chromium.org/buildbot/perf/xp-release-single-core/bloat-http/report.html?history=150
>>
>> But the memory test does use about 10% less RAM with jemalloc (as
>> predicted):
>>
>> http://build.chromium.org/buildbot/perf/vista-release-dual-core/memory/report.html?history=150
>> And almost all memory tests got better:
>>
>> http://build.chromium.org/buildbot/perf/dashboard/overview.html?graph=vm_peak_b
>>
>> So:  tcmalloc == faster; jemalloc == smaller.  We knew this.
>>
>> I think we should probably back out jemalloc - the perf hit is
>> non-trivial, and the memory we can improve with other, upcoming tcmalloc
>> work.  Jim - how badly do you want jemalloc dev-channel data?  I'm not sure
>> it will buy us a lot other than what we already know.
>>
>> Mike
>>
>>
>> On Thu, Oct 1, 2009 at 8:29 AM, Darin Fisher <[email protected]> wrote:
>>
>>> Note: my change was reverted for other reasons.-Darin
>>>
>>> On Thu, Oct 1, 2009 at 7:58 AM, Chase Phillips <[email protected]>wrote:
>>>
>>>> Hey everyone,
>>>> A performance regression has appeared on the XP Perf bot in the morejs
>>>> page cycler.  Along with turning XP Perf red, the perf regression system
>>>> notified me via email (forwarded below).  The page load regression is about
>>>> 30ms.  There's a similar regression in the moz page cycler.  Here's a link
>>>> to the XP Perf dashboard to see the morejs regression:
>>>>
>>>> http://build.chromium.org/buildbot/perf/xp-release-dual-core/morejs/report.html?history=50
>>>>
>>>> The interesting revisions between r27704 and r27710 are:
>>>>
>>>>   r27705 - darin - Move various methods from glue to 
>>>> api.<http://src.chromium.org/viewvc/chrome?view=rev&revision=27705>
>>>>   r27708 - jar - Set JEMalloc as the default allocator (instead of
>>>> TCMalloc).<http://src.chromium.org/viewvc/chrome?view=rev&revision=27708>
>>>>   r27710 - thestig - Disable CheckSvnModifiedDirectories for 
>>>> now.<http://src.chromium.org/viewvc/chrome?view=rev&revision=27710>
>>>>
>>>> @jar: Not meaning to pick on you, but my guess is the JEMalloc change is
>>>> the cause.  Did you expect a page load regression from your change?
>>>>
>>>> PS If you haven't heard of this perf regression system, don't worry --
>>>> it's a new tool in Chromium's toolbox.  Email and docs describing it will 
>>>> be
>>>> sent soon.
>>>>
>>>> Thanks,
>>>> Chase
>>>>
>>>> ---------- Forwarded message ----------
>>>> From: <[email protected]>
>>>> Date: Thu, Oct 1, 2009 at 7:15 AM
>>>> Subject: buildbot failure in Chromium on XP Perf, revision 27719
>>>> To: [email protected]
>>>>
>>>>
>>>>  http://build.chromium.org/buildbot/waterfall/
>>>>
>>>> Perf alert on "XP Perf": page_cycler_morejs
>>>>
>>>>
>>>> http://build.chromium.org/buildbot/waterfall/builders/XP%20Perf/builds/9414
>>>>
>>>> Revision: 27719
>>>> Blame list: [email protected]
>>>>
>>>>  XP Perf
>>>> Build 
>>>> 9414<http://build.chromium.org/buildbot/waterfall/builders/XP%20Perf/builds/9414>
>>>> svnkill
>>>> stdio<http://build.chromium.org/buildbot/waterfall/builders/XP%20Perf/builds/9414/steps/shell/logs/stdio>
>>>>   update
>>>> scripts
>>>> stdio<http://build.chromium.org/buildbot/waterfall/builders/XP%20Perf/builds/9414/steps/shell_2/logs/stdio>
>>>> taskkill
>>>> stdio<http://build.chromium.org/buildbot/waterfall/builders/XP%20Perf/builds/9414/steps/shell_3/logs/stdio>
>>>> update
>>>> stdio<http://build.chromium.org/buildbot/waterfall/builders/XP%20Perf/builds/9414/steps/gclient/logs/stdio>
>>>>   extract
>>>> build
>>>> stdio<http://build.chromium.org/buildbot/waterfall/builders/XP%20Perf/builds/9414/steps/extract%20build/logs/stdio>
>>>>   Start
>>>> Crash Handler
>>>> stdio<http://build.chromium.org/buildbot/waterfall/builders/XP%20Perf/builds/9414/steps/Start%20Crash%20Handler/logs/stdio>
>>>> uploading
>>>> perf_expectations.json  page_cycler_moz
>>>>
>>>> IO_b_b: 41.8k (39.7k)
>>>> IO_b_b_extcs1: 41.8k
>>>> IO_b_r: 6.74k (6.0k)
>>>> IO_b_r_extcs1: 6.62k
>>>> IO_op_b: 51.6k (51.8k)
>>>> IO_op_b_extcs1: 55.0k
>>>> IO_op_r: 25.3k (28.3k)
>>>> IO_op_r_extcs1: 25.2k
>>>> t: 1.08k (1.19k)
>>>> t_extcs1: 1.25k
>>>> vm_pk_b: 8.24M (17.0M)
>>>> vm_pk_b_extcs1: 9.68M
>>>> vm_pk_r: 83.2M (72.9M)
>>>> vm_pk_r_extcs1: 85.6M
>>>> ws_pk_b: 20.0M (24.8M)
>>>> ws_pk_b_extcs1: 21.4M
>>>> ws_pk_r: 75.3M (73.7M)
>>>> ws_pk_r_extcs1: 80.5M
>>>>
>>>> stdio<http://build.chromium.org/buildbot/waterfall/builders/XP%20Perf/builds/9414/steps/page_cycler_moz/logs/stdio>
>>>> [results<http://build.chromium.org/buildbot/perf/xp-release-dual-core/moz/report.html?history=150>
>>>> ]  page_cycler_morejs
>>>>
>>>> PERF_REGRESS: times/t
>>>> IO_b_b: 19.2k (17.5k)
>>>> IO_b_b_extcs1: 19.3k
>>>> IO_b_r: 475 (243)
>>>> IO_b_r_extcs1: 475
>>>> IO_op_b: 20.3k (19.5k)
>>>> IO_op_b_extcs1: 23.4k
>>>> IO_op_r: 9.56k (4.17k)
>>>> IO_op_r_extcs1: 9.56k
>>>> t: 1.4k (1.31k)
>>>> t_extcs1: 1.44k
>>>> vm_pk_b: 7.06M (16.1M)
>>>> vm_pk_b_extcs1: 8.49M
>>>> vm_pk_r: 8.28M (18.0M)
>>>> vm_pk_r_extcs1: 8.3M
>>>> ws_pk_b: 19.0M (23.8M)
>>>> ws_pk_b_extcs1: 20.4M
>>>> ws_pk_r: 12.6M (21.6M)
>>>> ws_pk_r_extcs1: 12.6M
>>>>
>>>> stdio<http://build.chromium.org/buildbot/waterfall/builders/XP%20Perf/builds/9414/steps/page_cycler_morejs/logs/stdio>
>>>> [results<http://build.chromium.org/buildbot/perf/xp-release-dual-core/morejs/report.html?history=150>
>>>> ]
>>>>
>>>> Changed by: *[email protected]*
>>>> Changed at: *Thu 01 Oct 2009 07:00:09*
>>>> Branch: *src*
>>>> Revision: *27719*
>>>>
>>>> Changed files:
>>>>
>>>>    - *chrome/browser/privacy_blacklist/blocked_response.cc*
>>>>    - *chrome/browser/privacy_blacklist/blocked_response.h*
>>>>    - *chrome/browser/resources/privacy_blacklist_block.html*
>>>>    - *chrome/browser/renderer_host/resource_dispatcher_host.cc*
>>>>    - *chrome/browser/renderer_host/resource_dispatcher_host.h*
>>>>
>>>> Comments:
>>>>
>>>> Privacy Blacklist Unblock
>>>>
>>>> Summary
>>>> -------
>>>>
>>>> Mostly implemented the unblocking for visual resources for the Privacy 
>>>> Blacklist.
>>>> Merging now before I leave. Eveything here only has effect if the 
>>>> --privacy-blacklist
>>>> flag specifies a Privacy Blacklist.
>>>>
>>>> Detailed Changes
>>>> ----------------
>>>>
>>>> [chrome/browser/resources/privacy_blacklist.html]
>>>>
>>>> - Replaced the about:blank place-holder with variable to set the unblock 
>>>> link.
>>>>
>>>> - Open the Privacy Blacklist provider page in a new tab. This works around 
>>>> an
>>>>   issue where such request for a full-page (rather than a sub-resource) 
>>>> gets
>>>>   blocked indefinitely.
>>>>
>>>> [chrome/browser/render_host/resource_dispatcher_host.h]
>>>>
>>>> - Added a BlockedResponse member which is now a class rather than a 
>>>> namespace,
>>>>   see below for more information.
>>>>
>>>> [chrome/browser/render_host/resource_dispatcher_host.cc]
>>>>
>>>> - Generate headers for the blocked response to redirect to the 
>>>> chrome-blocked URL
>>>>   which prevents an enclosing page from reading the URL of the unblock 
>>>> link. This
>>>>   was suggested by Darin to avoid scripted bypassing of blocked contents.
>>>>
>>>> - Recover the original URL for blocked content, in order to fetch it during
>>>>   unblocking.
>>>>
>>>> - Do not create CrossSiteResourceHandler when an unblocked link is 
>>>> requested.
>>>>   Otherwise the request never resumes as the blocked page never gets closed
>>>>   since it is not a real page.
>>>>
>>>> [chrome/browser/privacy_blacklist/blocked_response.cc]
>>>>
>>>> - Defined chrome-block and chrome-unblock URL schemes. The block scheme is 
>>>> used
>>>>   to return the blocked response. The unblock scheme is used request a 
>>>> blocked
>>>>   resource's URL without being intercepted by the Privacy Blacklist.
>>>>
>>>> - Defined a hash function for a blocked resource as its address in memory.
>>>>   Function to reverse the hash is therefore trivial.
>>>>
>>>> - Added a function to return headers for a blocked response.
>>>>
>>>> - Added a function to generate a block URL from a requested one.
>>>>
>>>> - Added a function to get an unblock URL from a requested one.
>>>>
>>>> - Added a function to return the original URL for a blocked one.
>>>>
>>>> [chrome/browser/privacy_blacklist/blocked_response.h]
>>>>
>>>> - Made the BlockedResponse namespace into a class.
>>>>
>>>> - Created a member set to keep all the blocked resources URL.
>>>>
>>>> BUG=16932
>>>> TEST=none
>>>> TBR=darin
>>>>
>>>> Review URL: http://codereview.chromium.org/252001
>>>>
>>>>
>>>>
>>>
>>>
>>>
>
> >
>

--~--~---------~--~----~------------~-------~--~----~
Chromium Developers mailing list: [email protected] 
View archives, change email options, or unsubscribe: 
    http://groups.google.com/group/chromium-dev
-~----------~----~----~----~------~----~------~--~---

Reply via email to