27831 2009-07-30 05:18:35 -0700 Allow custom memory allocation control for JavaScriptCore's UString class 2011-06-17 01:05:42 -0700 1 1 1 Unclassified WebKit JavaScriptCore 528+ (Nightly build) PC All RESOLVED INVALID P2 Normal --- 1 zoltan webkit-unassigned andersca barraclough commit-queue darin eric ggaren mjs oldest_to_newest 135820 0 zoltan 2009-07-30 05:18:35 -0700 Inherits UString class from FastAllocBase because it has been instantiated by 'new' in JavaScriptCore/runtime/UString.cpp:212. 135821 1 33770 zoltan 2009-07-30 05:21:54 -0700 Created attachment 33770 proposed patch 136262 2 33770 eric 2009-07-31 15:09:34 -0700 Comment on attachment 33770 proposed patch Did you run the layout tests? 11027 test cases (99%) succeeded 1 test case (<1%) timed out 5 test cases (<1%) crashed 14 test cases (<1%) had stderr output 144121 3 zoltan 2009-09-02 00:14:51 -0700 This patch breaks the layout tests on mac me too. I mark it as invalid. 181787 4 zoltan 2010-01-18 01:50:15 -0800 I tested it again, now the layout tests work well. 181789 5 46805 zoltan 2010-01-18 01:53:49 -0800 Created attachment 46805 updated patch 181876 6 46805 aroben 2010-01-18 07:44:14 -0800 Comment on attachment 46805 updated patch This patch changes JSC::CString, not JSC::UString as the bug and ChangeLog suggest. What's really going on here? 181908 7 46831 zoltan 2010-01-18 09:58:14 -0800 Created attachment 46831 updated proposed patch I just uploaded from wrong directory. This is the good one. 182038 8 46831 commit-queue 2010-01-18 16:51:04 -0800 Comment on attachment 46831 updated proposed patch Clearing flags on attachment: 46831 Committed r53438: <http://trac.webkit.org/changeset/53438> 182039 9 commit-queue 2010-01-18 16:51:13 -0800 All reviewed patches have been landed. Closing bug. 182050 10 darin 2010-01-18 17:17:51 -0800 I have bad news. First, I have to roll this out because adding FastAllocBase as a base class made UString bigger than a single pointer, and in turn that made JSString too big for a garbage collection cell. Second, that means that it’s likely that adding FastAllocBase as base class for other objects has been making them all larger, inflating WebKit’s memory use. Someone needs to investigate this further. 182052 11 darin 2010-01-18 17:20:09 -0800 Rolled out in <http://trac.webkit.org/changeset/53438>. 182058 12 darin 2010-01-18 17:26:55 -0800 Anders Carlsson explained to me what’s going wrong. The problem is that RefPtr derives from FastAllocBase and so does UString. Since the first member of UString is a RefPtr, it can’t be at the same address of the FastAllocBase we are inheriting from. So padding is added, making the object bigger. This will happen any time the first data member of a class is an object of a class that derives from FastAllocBase (or Noncopyable or anything else that eventually leads up to FastAllocBase), and the class itself also derives from FastAllocBase (or Noncopyable, etc.). One way to eliminate the problem is to make FastAllocBase a class template and inherit from FastAllocBase<X> so that each use of FastAllocBase is a unique class, but this still won’t help in the case of Noncopyable. We may want to file a new bug about this. FastAllocBase is probably making many of our objects a bit bigger. 182060 13 darin 2010-01-18 17:35:32 -0800 I believe the build was broken only on 64-bit platforms, so the SnowLeopard bot showed the problem but the Leopard one did not. 182072 14 eric 2010-01-18 17:58:21 -0800 The commit-queue really needs to be taught how to roll out its own patches whne it turns the bots red. Sorry about the break. :( 182763 15 zoltan 2010-01-20 06:13:47 -0800 (In reply to comment #10) > First, I have to roll this out because adding FastAllocBase as a base class > made UString bigger than a single pointer, and in turn that made JSString too > big for a garbage collection cell. I didn't know that UString is a GC collected class . Where is it converted to JSString? I opened a new bug "FastAllocBase is probably making many of our objects a bit bigger" bug #33896. 182799 16 darin 2010-01-20 07:51:01 -0800 (In reply to comment #15) > I didn't know that UString is a GC collected class . Where is it converted to > JSString? It's not a GC-collected class. But JSString objects have a UString as a data member. You can find it in JSString.h. 422412 17 barraclough 2011-06-16 21:52:44 -0700 We no longer use FastAllocBase, and we do not heap allocate individual UString objects (it is just a pointer). This patch does not seem valid. 422477 18 zoltan 2011-06-17 01:05:42 -0700 (In reply to comment #17) > We no longer use FastAllocBase, and we do not heap allocate individual UString objects (it is just a pointer). This patch does not seem valid. Yes, exactly! It has been rolled out in <http://trac.webkit.org/changeset/53438>. 33770 2009-07-30 05:21:54 -0700 2010-01-18 01:53:49 -0800 proposed patch UString.patch text/plain 1120 zoltan SW5kZXg6IEphdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBKYXZhU2NyaXB0 Q29yZS9DaGFuZ2VMb2cJKHJldmlzaW9uIDQ2NTc1KQorKysgSmF2YVNjcmlwdENvcmUvQ2hhbmdl TG9nCSh3b3JraW5nIGNvcHkpCkBAIC0xLDMgKzEsMTUgQEAKKzIwMDktMDctMzAgWm9sdGFuIEhv cnZhdGggIDxoem9sdGFuQGluZi51LXN6ZWdlZC5odT4KKworICAgICAgICBSZXZpZXdlZCBieSBO T0JPRFkgKE9PUFMhKS4KKworICAgICAgICBBbGxvdyBjdXN0b20gbWVtb3J5IGFsbG9jYXRpb24g Y29udHJvbCBmb3IgSmF2YVNjcmlwdENvcmUncyBVU3RyaW5nIGNsYXNzCisgICAgICAgIGh0dHBz Oi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0yNzgzMQorCisgICAgICAgIEluaGVy aXRzIFVTdHJpbmcgY2xhc3MgZnJvbSBGYXN0QWxsb2NCYXNlIGJlY2F1c2UgaXQgaGFzIGJlZW4K KyAgICAgICAgaW5zdGFudGlhdGVkIGJ5ICduZXcnIGluIEphdmFTY3JpcHRDb3JlL3J1bnRpbWUv VVN0cmluZy5jcHA6MjEyLgorCisgICAgICAgICogcnVudGltZS9VU3RyaW5nLmg6CisKIDIwMDkt MDctMjkgIExhc3psbyBHb21ib3MgIDxsYXN6bG8uMS5nb21ib3NAbm9raWEuY29tPgogCiAgICAg ICAgIFJldmlld2VkIGJ5IEdlb3JnZSBTdGFpa29zLgpJbmRleDogSmF2YVNjcmlwdENvcmUvcnVu dGltZS9VU3RyaW5nLmgKPT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gSmF2YVNjcmlwdENvcmUvcnVudGltZS9VU3Ry aW5nLmgJKHJldmlzaW9uIDQ2NTc1KQorKysgSmF2YVNjcmlwdENvcmUvcnVudGltZS9VU3RyaW5n LmgJKHdvcmtpbmcgY29weSkKQEAgLTczLDcgKzczLDcgQEAgbmFtZXNwYWNlIEpTQyB7CiAKICAg ICB0eXBlZGVmIFZlY3RvcjxjaGFyLCAzMj4gQ1N0cmluZ0J1ZmZlcjsKIAotICAgIGNsYXNzIFVT dHJpbmcgeworICAgIGNsYXNzIFVTdHJpbmcgOiBwdWJsaWMgRmFzdEFsbG9jQmFzZSB7CiAgICAg ICAgIGZyaWVuZCBjbGFzcyBKSVQ7CiAKICAgICBwdWJsaWM6Cg== 46805 2010-01-18 01:53:49 -0800 2010-01-18 09:58:14 -0800 updated patch ustring2.patch text/plain 1218 zoltan SW5kZXg6IEphdmFTY3JpcHRDb3JlL3J1bnRpbWUvVVN0cmluZy5oCj09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIEph dmFTY3JpcHRDb3JlL3J1bnRpbWUvVVN0cmluZy5oCShyZXZpc2lvbiA1MzM5NCkKKysrIEphdmFT Y3JpcHRDb3JlL3J1bnRpbWUvVVN0cmluZy5oCSh3b3JraW5nIGNvcHkpCkBAIC00MCw3ICs0MCw3 IEBACiAgICAgdXNpbmcgV1RGOjpQbGFjZW1lbnROZXdBZG9wdFR5cGU7CiAgICAgdXNpbmcgV1RG OjpQbGFjZW1lbnROZXdBZG9wdDsKIAotICAgIGNsYXNzIENTdHJpbmcgeworICAgIGNsYXNzIENT dHJpbmcgOiBwdWJsaWMgRmFzdEFsbG9jQmFzZSB7CiAgICAgcHVibGljOgogICAgICAgICBDU3Ry aW5nKCkKICAgICAgICAgICAgIDogbV9sZW5ndGgoMCkKSW5kZXg6IEphdmFTY3JpcHRDb3JlL0No YW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09Ci0tLSBKYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cJKHJldmlzaW9u IDUzMzk0KQorKysgSmF2YVNjcmlwdENvcmUvQ2hhbmdlTG9nCSh3b3JraW5nIGNvcHkpCkBAIC0x LDMgKzEsMTggQEAKKzIwMTAtMDEtMTggIFpvbHRhbiBIb3J2YXRoICA8em9sdGFuQHdlYmtpdC5v cmc+CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgQWxs b3cgY3VzdG9tIG1lbW9yeSBhbGxvY2F0aW9uIGNvbnRyb2wgZm9yIFVTdHJpbmcgY2xhc3MKKyAg ICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTI3ODMxCisKKyAg ICAgICAgSW5oZXJpdHMgdGhlIGZvbGxvd2luZyBjbGFzcyBmcm9tIEZhc3RBbGxvY0Jhc2UgYmVj YXVzZSBpdCBpcworICAgICAgICBpbnN0YW50aWF0ZWQgYnkgJ25ldycgYW5kIG5vIG5lZWQgdG8g YmUgY29weWFibGU6CisKKyAgICAgICAgY2xhc3MgbmFtZSAgICAgLSBpbnN0YW50aWF0ZWQgYXQ6 CisgICAgICAgIGNsYXNzcyBVU3RyaW5nIC0gSmF2YVNjcmlwdENvcmUvcnVudGltZS9VU3RyaW5n LmNwcDoxNjAKKworICAgICAgICAqIHJ1bnRpbWUvVVN0cmluZy5oOgorCiAyMDEwLTAxLTE1ICBH YXZpbiBCYXJyYWNsb3VnaCAgPGJhcnJhY2xvdWdoQGFwcGxlLmNvbT4KIAogICAgICAgICBSZXZp ZXdlZCBieSBPbGl2ZXIgSHVudC4K 46831 2010-01-18 09:58:14 -0800 2010-06-10 20:33:43 -0700 updated proposed patch ustring3.patch text/plain 1174 zoltan SW5kZXg6IEphdmFTY3JpcHRDb3JlL3J1bnRpbWUvVVN0cmluZy5oCj09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIEph dmFTY3JpcHRDb3JlL3J1bnRpbWUvVVN0cmluZy5oCShyZXZpc2lvbiA1MzM5NCkKKysrIEphdmFT Y3JpcHRDb3JlL3J1bnRpbWUvVVN0cmluZy5oCSh3b3JraW5nIGNvcHkpCkBAIC03Myw3ICs3Myw3 IEBACiAKICAgICB0eXBlZGVmIFZlY3RvcjxjaGFyLCAzMj4gQ1N0cmluZ0J1ZmZlcjsKIAotICAg IGNsYXNzIFVTdHJpbmcgeworICAgIGNsYXNzIFVTdHJpbmcgOiBwdWJsaWMgRmFzdEFsbG9jQmFz ZSB7CiAgICAgICAgIGZyaWVuZCBjbGFzcyBKSVQ7CiAKICAgICBwdWJsaWM6CkluZGV4OiBKYXZh U2NyaXB0Q29yZS9DaGFuZ2VMb2cKPT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gSmF2YVNjcmlwdENvcmUvQ2hhbmdl TG9nCShyZXZpc2lvbiA1MzM5NCkKKysrIEphdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwkod29ya2lu ZyBjb3B5KQpAQCAtMSwzICsxLDE4IEBACisyMDEwLTAxLTE4ICBab2x0YW4gSG9ydmF0aCAgPHpv bHRhbkB3ZWJraXQub3JnPgorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgor CisgICAgICAgIEFsbG93IGN1c3RvbSBtZW1vcnkgYWxsb2NhdGlvbiBjb250cm9sIGZvciBVU3Ry aW5nIGNsYXNzCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9p ZD0yNzgzMQorCisgICAgICAgIEluaGVyaXRzIHRoZSBmb2xsb3dpbmcgY2xhc3MgZnJvbSBGYXN0 QWxsb2NCYXNlIGJlY2F1c2UgaXQgaXMKKyAgICAgICAgaW5zdGFudGlhdGVkIGJ5ICduZXcnIGFu ZCBubyBuZWVkIHRvIGJlIGNvcHlhYmxlOgorCisgICAgICAgIGNsYXNzIG5hbWUgICAgIC0gaW5z dGFudGlhdGVkIGF0OgorICAgICAgICBjbGFzc3MgVVN0cmluZyAtIEphdmFTY3JpcHRDb3JlL3J1 bnRpbWUvVVN0cmluZy5jcHA6MTYwCisKKyAgICAgICAgKiBydW50aW1lL1VTdHJpbmcuaDoKKwog MjAxMC0wMS0xNSAgR2F2aW4gQmFycmFjbG91Z2ggIDxiYXJyYWNsb3VnaEBhcHBsZS5jb20+CiAK ICAgICAgICAgUmV2aWV3ZWQgYnkgT2xpdmVyIEh1bnQuCg==