Page MenuHomePhabricator

ezbr (Evan Brown)
User

Projects

User does not belong to any projects.

User Details

User Since
May 17 2021, 7:40 AM (11 w, 21 h)

Google software engineer focused on efficiency

Recent Activity

Thu, Jul 29

ezbr added a comment to D102727: Use precise growth rather than amortized growth in std::string::resize/__resize_default_init..

Could you rebase your patch on main? Sorry for the inconvenience.

Done

Thu, Jul 29, 1:44 PM · Restricted Project
ezbr updated the summary of D102727: Use precise growth rather than amortized growth in std::string::resize/__resize_default_init..
Thu, Jul 29, 1:42 PM · Restricted Project
ezbr updated the summary of D102727: Use precise growth rather than amortized growth in std::string::resize/__resize_default_init..
Thu, Jul 29, 1:42 PM · Restricted Project
ezbr updated the diff for D102727: Use precise growth rather than amortized growth in std::string::resize/__resize_default_init..

Remove ifdef guards (so we change the default behavior) and rebase onto a newer version of llvm.

Thu, Jul 29, 1:41 PM · Restricted Project

Wed, Jul 21

ezbr updated the diff for D102727: Use precise growth rather than amortized growth in std::string::resize/__resize_default_init..

Update config comment and change test to not hardcode basic_string::alignment.

Wed, Jul 21, 11:42 AM · Restricted Project
ezbr added a comment to D102727: Use precise growth rather than amortized growth in std::string::resize/__resize_default_init..

Thanks for your replies. I'd like to understand: how did you measure the memory improvements you found? Ideally, it would be nice to have something that we can reproduce on a large open-source code base (e.g. Chrome) so that the experiment can be verified. Is that possible? Otherwise, you have to understand I'm blindly basing the decision on "an internal Google experiment", which while I do trust, is not the best from a due diligence perspective. I'm being very conservative because silently introducing quadratic behavior in user code is pretty close to the scariest thing a maintainer can think about -- very bad but also very difficult to detect.

Wed, Jul 21, 11:41 AM · Restricted Project

Tue, Jul 20

ezbr added a comment to D102727: Use precise growth rather than amortized growth in std::string::resize/__resize_default_init..

Regarding kAlign, this is basic_string::__alignment, which is private. Do you think it's okay to include this implementation detail here? If not, I think it could be computed, but I thought that could add complexity to the test and that this value is unlikely to change often. Do you prefer computing it in the test?

Tue, Jul 20, 10:39 AM · Restricted Project
ezbr updated the diff for D102727: Use precise growth rather than amortized growth in std::string::resize/__resize_default_init..

Add wstring coverage and add a comment in the test.

Tue, Jul 20, 10:34 AM · Restricted Project
ezbr added a comment to D102727: Use precise growth rather than amortized growth in std::string::resize/__resize_default_init..

I followed @Quuxplusone's recommendation of using a new __config flag and enabling it by default for unstable ABI/ABI version 2.

Tue, Jul 20, 9:05 AM · Restricted Project
ezbr updated the diff for D102727: Use precise growth rather than amortized growth in std::string::resize/__resize_default_init..

Fix an issue with the previous version of the review.

Tue, Jul 20, 9:02 AM · Restricted Project

Mon, Jul 19

ezbr added a comment to D102727: Use precise growth rather than amortized growth in std::string::resize/__resize_default_init..

Somehow the current version of this phabricator review doesn't seem to include my changes other than the new test file. I'll try to figure it out tomorrow.

Mon, Jul 19, 3:26 PM · Restricted Project
ezbr updated the diff for D102727: Use precise growth rather than amortized growth in std::string::resize/__resize_default_init..

Try again to update phabricator review. The previous attempt somehow only had the test file change.

Mon, Jul 19, 3:18 PM · Restricted Project
ezbr added a comment to D102727: Use precise growth rather than amortized growth in std::string::resize/__resize_default_init..

I like the way this change is done better now, but I'm still not sure we want/can do this -- I have a few questions I need the answer to before this LGTM. Sorry if that sounds like I'm trying to poke holes in your optimization, but doing my due diligence requires me to try for a type as important as std::string and a method as widely used as resize(). We really don't want to get this wrong.

  1. Does this change how iterators are invalidated?
Mon, Jul 19, 3:06 PM · Restricted Project
ezbr updated the diff for D102727: Use precise growth rather than amortized growth in std::string::resize/__resize_default_init..

Guard changes behind new _LIBCPP_ABI_EXACT_STRING_RESIZE config flag and add a test.

Mon, Jul 19, 3:05 PM · Restricted Project
ezbr updated the summary of D102727: Use precise growth rather than amortized growth in std::string::resize/__resize_default_init..
Mon, Jul 19, 2:57 PM · Restricted Project

Wed, Jul 14

ezbr updated the summary of D102727: Use precise growth rather than amortized growth in std::string::resize/__resize_default_init..
Wed, Jul 14, 3:04 PM · Restricted Project
ezbr added a comment to D102727: Use precise growth rather than amortized growth in std::string::resize/__resize_default_init..

If this is only an experiment and we're not sure we want to keep it, I would much prefer that you enable it internally in your downstream fork and then upstream it alongside with the data you gathered if the experiment turns out to be positive. When we add options to libc++, people are free to start using them and they are a lot harder to remove. I'd be happy to make this the default in libc++ if it's shown to be an improvement and keeps us conforming.

Wed, Jul 14, 2:56 PM · Restricted Project
ezbr updated the summary of D102727: Use precise growth rather than amortized growth in std::string::resize/__resize_default_init..
Wed, Jul 14, 2:55 PM · Restricted Project
ezbr retitled D102727: Use precise growth rather than amortized growth in std::string::resize/__resize_default_init. from Add a compiler option to make string resize/__resize_default_init use precise growth instead of amortized growth. to Use precise growth rather than amortized growth in std::string::resize/__resize_default_init..
Wed, Jul 14, 2:51 PM · Restricted Project
ezbr updated the diff for D102727: Use precise growth rather than amortized growth in std::string::resize/__resize_default_init..

Change from adding an option for precise resize to doing so by default and update the change description.

Wed, Jul 14, 2:49 PM · Restricted Project

Jun 16 2021

ezbr added a comment to D102727: Use precise growth rather than amortized growth in std::string::resize/__resize_default_init..

I updated the documentation and added more details about the motivation to the revision summary.

Jun 16 2021, 2:09 PM · Restricted Project
ezbr updated the summary of D102727: Use precise growth rather than amortized growth in std::string::resize/__resize_default_init..
Jun 16 2021, 2:07 PM · Restricted Project
ezbr updated the diff for D102727: Use precise growth rather than amortized growth in std::string::resize/__resize_default_init..

Updated configuration macro documentation.

Jun 16 2021, 2:01 PM · Restricted Project

May 18 2021

ezbr requested review of D102727: Use precise growth rather than amortized growth in std::string::resize/__resize_default_init..
May 18 2021, 2:37 PM · Restricted Project
ezbr updated ezbr.
May 18 2021, 11:12 AM