Closed Bug 1128732 Opened 6 years ago Closed 6 years ago
39 bytes, text/x-review-board-request
Somehow this got all crufty. Easily fixed.
/r/3305 - Bug 1128732 - Simplify js::DefineProperties. r?efaust. Pull down this commit: hg pull review -r df69ea5733b6c288060e01c0b0ea1fb8947ab8f4
Attachment #8558884 - Flags: review?(efaustbmo)
This reverses changes that happened in bug 885729. In my view, those changes were a mistake for a couple of reasons. First, we probably should never take patches that add code solely for performance reasons without at least crude measurements. (I admit I occasionally get the temptation myself.) At least we should be strongly prejudiced against taking such patches. In this particular case, I also don't imagine that patch was all that effective, because: 1. ReadPropertyDescriptors probably dominates the runtime. 2. Object.defineProperties on an array is probably vanishingly rare, even in the future. 3. The speed of Object.defineProperties on a proxy probably wasn't measurably improved by the patch, given that it only removes a branch or two and an indirect function call per property defined. Compared to the proxy overhead still incurred, that's a pretty tiny win.
https://reviewboard.mozilla.org/r/3305/#review2715 Yeah, no need to pull special cases on top of special cases...
Comment on attachment 8558884 [details] MozReview Request: bz://1128732/jorendorff Reviewboard snafu. This was already reviewed and landed.
Attachment #8558884 - Flags: review?(efaustbmo) → review+
You need to log in before you can comment on or make changes to this bug.