125000 2013-11-28 23:14:51 -0800 [WebGL] Wrong condition order in the if statement 2014-01-13 03:29:46 -0800 1 1 1 Unclassified WebKit WebGL 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 p.szymanski3 webkit-unassigned bfulgham commit-queue dino kondapallykalyan noam roger_fong oldest_to_newest 954887 0 p.szymanski3 2013-11-28 23:14:51 -0800 We should make a more efficient if statements. In this case bool variable should be checked before string comparison. Only in one case (first check) a string variable will be compare. After that a bool variable is set and string should not be compare again. 954909 1 218042 p.szymanski3 2013-11-29 01:51:24 -0800 Created attachment 218042 patch 966176 2 218042 bfulgham 2014-01-09 19:34:52 -0800 Comment on attachment 218042 patch View in context: https://bugs.webkit.org/attachment.cgi?id=218042&action=review You are right! Let's clean the change log up slightly and get this landed. > Source/WebCore/ChangeLog:3 > + [WebGL] Wrong condition order in the if statement [WebGL] Revise condition order for better performance > Source/WebCore/ChangeLog:12 > + this method. Please Rephrase: We should revise the logic so that we only pay the cost of string comparison once. The current logic causes the string comparison to happen every time we enter this function. Instead, by reordering the statements we can do a simple boolean test after the initial setup is complete. 966413 3 218042 bfulgham 2014-01-10 10:02:16 -0800 Comment on attachment 218042 patch r=me. I'll fix the Changelog while landing. 966416 4 bfulgham 2014-01-10 10:08:11 -0800 Committed r161645: <http://trac.webkit.org/changeset/161645> 967234 5 p.szymanski3 2014-01-13 03:29:46 -0800 (In reply to comment #3) > (From update of attachment 218042 [details]) > r=me. I'll fix the Changelog while landing. Thank you very much for review of my patches. Unfortunately I didn't have time for update this commit message. Please be my guest to do this. cq? 218042 2013-11-29 01:51:24 -0800 2014-01-10 10:02:16 -0800 patch framebuffer_condition_order.patch text/plain 3447 p.szymanski3 ZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZyBiL1NvdXJjZS9XZWJDb3JlL0No YW5nZUxvZwppbmRleCBkMDMzNDIyLi43NzMxNzgzIDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViQ29y ZS9DaGFuZ2VMb2cKKysrIGIvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMjEg QEAKKzIwMTMtMTEtMjkgIFByemVteXNsYXcgU3p5bWFuc2tpICA8cC5zenltYW5za2kzQHNhbXN1 bmcuY29tPgorCisgICAgICAgIFtXZWJHTF0gV3JvbmcgY29uZGl0aW9uIG9yZGVyIGluIHRoZSBp ZiBzdGF0ZW1lbnQKKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dp P2lkPTEyNTAwMAorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAg ICAgIEluIHRoaXMgY2FzZSBzdHJpbmcgY29tcGFyaXNvbiBzaG91bGQgYmUgZG9uZSBvbmx5IG9u ZSB0aW1lLgorICAgICAgICBBZnRlciB0aGF0IG9ubHkgYm9vbCBzdGF0ZW1lbnQgc2hvdWxkIGJl IGNoZWNrZWQuIFN0cmluZworICAgICAgICBjb21wYXJpc29uIGlzIG11Y2ggc2xvd2VyIChkZXBl bmRzIG9uIHN0cmluZyBsZW5ndGgpIHRoYW4KKyAgICAgICAgYm9vbCBjb21wYXJlIGFuZCBpdCBp cyBub3QgbmVjZXNzYXJ5IGluIHRoZSBmdXR1cmUgdXNlIG9mCisgICAgICAgIHRoaXMgbWV0aG9k LgorCisgICAgICAgIE5vIG5ldyB0ZXN0cy4gTm8gYmVoYXZpb3VyIGNoYW5nZXMuCisKKyAgICAg ICAgKiBwbGF0Zm9ybS9ncmFwaGljcy9vcGVuZ2wvRXh0ZW5zaW9uczNET3BlbkdMRVMuY3BwOgor ICAgICAgICAoV2ViQ29yZTo6RXh0ZW5zaW9uczNET3BlbkdMRVM6OnN1cHBvcnRzRXh0ZW5zaW9u KToKKwogMjAxMy0xMS0yOCAgR3l1eW91bmcgS2ltICA8Z3l1eW91bmcua2ltQHNhbXN1bmcuY29t PgogCiAgICAgICAgIFJlbmFtZSBJbmxpbmVJdGVyYXRvcjo6bV9vYmogYW5kIG1ha2UgaXQgcHJp dmF0ZQpkaWZmIC0tZ2l0IGEvU291cmNlL1dlYkNvcmUvcGxhdGZvcm0vZ3JhcGhpY3Mvb3Blbmds L0V4dGVuc2lvbnMzRE9wZW5HTEVTLmNwcCBiL1NvdXJjZS9XZWJDb3JlL3BsYXRmb3JtL2dyYXBo aWNzL29wZW5nbC9FeHRlbnNpb25zM0RPcGVuR0xFUy5jcHAKaW5kZXggNjYyYmIwMi4uMjNmM2I5 YyAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvcGxhdGZvcm0vZ3JhcGhpY3Mvb3BlbmdsL0V4 dGVuc2lvbnMzRE9wZW5HTEVTLmNwcAorKysgYi9Tb3VyY2UvV2ViQ29yZS9wbGF0Zm9ybS9ncmFw aGljcy9vcGVuZ2wvRXh0ZW5zaW9uczNET3BlbkdMRVMuY3BwCkBAIC0yNDAsMTcgKzI0MCwxNyBA QCB2b2lkIEV4dGVuc2lvbnMzRE9wZW5HTEVTOjpnZXRuVW5pZm9ybWl2RVhUKEdDM0R1aW50IHBy b2dyYW0sIGludCBsb2NhdGlvbiwgR0MzRAogYm9vbCBFeHRlbnNpb25zM0RPcGVuR0xFUzo6c3Vw cG9ydHNFeHRlbnNpb24oY29uc3QgU3RyaW5nJiBuYW1lKQogewogICAgIGlmIChtX2F2YWlsYWJs ZUV4dGVuc2lvbnMuY29udGFpbnMobmFtZSkpIHsKLSAgICAgICAgaWYgKG5hbWUgPT0gIkdMX09F U192ZXJ0ZXhfYXJyYXlfb2JqZWN0IiAmJiAhbV9zdXBwb3J0c09FU3ZlcnRleEFycmF5T2JqZWN0 KSB7CisgICAgICAgIGlmICghbV9zdXBwb3J0c09FU3ZlcnRleEFycmF5T2JqZWN0ICYmIG5hbWUg PT0gIkdMX09FU192ZXJ0ZXhfYXJyYXlfb2JqZWN0IikgewogICAgICAgICAgICAgbV9nbEJpbmRW ZXJ0ZXhBcnJheU9FUyA9IHJlaW50ZXJwcmV0X2Nhc3Q8UEZOR0xCSU5EVkVSVEVYQVJSQVlPRVNQ Uk9DPihlZ2xHZXRQcm9jQWRkcmVzcygiZ2xCaW5kVmVydGV4QXJyYXlPRVMiKSk7CiAgICAgICAg ICAgICBtX2dsR2VuVmVydGV4QXJyYXlzT0VTID0gcmVpbnRlcnByZXRfY2FzdDxQRk5HTEdFTlZF UlRFWEFSUkFZU09FU1BST0M+KGVnbEdldFByb2NBZGRyZXNzKCJnbEdlblZlcnRleEFycmF5c09F UyIpKTsKICAgICAgICAgICAgIG1fZ2xEZWxldGVWZXJ0ZXhBcnJheXNPRVMgPSByZWludGVycHJl dF9jYXN0PFBGTkdMREVMRVRFVkVSVEVYQVJSQVlTT0VTUFJPQz4oZWdsR2V0UHJvY0FkZHJlc3Mo ImdsRGVsZXRlVmVydGV4QXJyYXlzT0VTIikpOwogICAgICAgICAgICAgbV9nbElzVmVydGV4QXJy YXlPRVMgPSByZWludGVycHJldF9jYXN0PFBGTkdMSVNWRVJURVhBUlJBWU9FU1BST0M+KGVnbEdl dFByb2NBZGRyZXNzKCJnbElzVmVydGV4QXJyYXlPRVMiKSk7CiAgICAgICAgICAgICBtX3N1cHBv cnRzT0VTdmVydGV4QXJyYXlPYmplY3QgPSB0cnVlOwotICAgICAgICB9IGVsc2UgaWYgKG5hbWUg PT0gIkdMX0lNR19tdWx0aXNhbXBsZWRfcmVuZGVyX3RvX3RleHR1cmUiICYmICFtX3N1cHBvcnRz SU1HTXVsdGlzYW1wbGVkUmVuZGVyVG9UZXh0dXJlKSB7CisgICAgICAgIH0gZWxzZSBpZiAoIW1f c3VwcG9ydHNJTUdNdWx0aXNhbXBsZWRSZW5kZXJUb1RleHR1cmUgJiYgbmFtZSA9PSAiR0xfSU1H X211bHRpc2FtcGxlZF9yZW5kZXJfdG9fdGV4dHVyZSIpIHsKICAgICAgICAgICAgIG1fZ2xGcmFt ZWJ1ZmZlclRleHR1cmUyRE11bHRpc2FtcGxlSU1HID0gcmVpbnRlcnByZXRfY2FzdDxQRk5HTEZS QU1FQlVGRkVSVEVYVFVSRTJETVVMVElTQU1QTEVJTUc+KGVnbEdldFByb2NBZGRyZXNzKCJnbEZy YW1lYnVmZmVyVGV4dHVyZTJETXVsdGlzYW1wbGVJTUciKSk7CiAgICAgICAgICAgICBtX2dsUmVu ZGVyYnVmZmVyU3RvcmFnZU11bHRpc2FtcGxlSU1HID0gcmVpbnRlcnByZXRfY2FzdDxQRk5HTFJF TkRFUkJVRkZFUlNUT1JBR0VNVUxUSVNBTVBMRUlNRz4oZWdsR2V0UHJvY0FkZHJlc3MoImdsUmVu ZGVyYnVmZmVyU3RvcmFnZU11bHRpc2FtcGxlSU1HIikpOwogICAgICAgICAgICAgbV9zdXBwb3J0 c0lNR011bHRpc2FtcGxlZFJlbmRlclRvVGV4dHVyZSA9IHRydWU7Ci0gICAgICAgIH0gZWxzZSBp ZiAobmFtZSA9PSAiR0xfRVhUX3JvYnVzdG5lc3MiICYmICFtX2dsR2V0R3JhcGhpY3NSZXNldFN0 YXR1c0VYVCkgeworICAgICAgICB9IGVsc2UgaWYgKCFtX2dsR2V0R3JhcGhpY3NSZXNldFN0YXR1 c0VYVCAmJiBuYW1lID09ICJHTF9FWFRfcm9idXN0bmVzcyIpIHsKICAgICAgICAgICAgIG1fZ2xH ZXRHcmFwaGljc1Jlc2V0U3RhdHVzRVhUID0gcmVpbnRlcnByZXRfY2FzdDxQRk5HTEdFVEdSQVBI SUNTUkVTRVRTVEFUVVNFWFRQUk9DPihlZ2xHZXRQcm9jQWRkcmVzcygiZ2xHZXRHcmFwaGljc1Jl c2V0U3RhdHVzRVhUIikpOwogICAgICAgICAgICAgbV9nbFJlYWRuUGl4ZWxzRVhUID0gcmVpbnRl cnByZXRfY2FzdDxQRk5HTFJFQUROUElYRUxTRVhUUFJPQz4oZWdsR2V0UHJvY0FkZHJlc3MoImds UmVhZG5QaXhlbHNFWFQiKSk7CiAgICAgICAgICAgICBtX2dsR2V0blVuaWZvcm1mdkVYVCA9IHJl aW50ZXJwcmV0X2Nhc3Q8UEZOR0xHRVROVU5JRk9STUZWRVhUUFJPQz4oZWdsR2V0UHJvY0FkZHJl c3MoImdsR2V0blVuaWZvcm1mdkVYVCIpKTsK