https://codereview.chromium.org/14779011/diff/6001/src/object-observe.js
File src/object-observe.js (right):
https://codereview.chromium.org/14779011/diff/6001/src/object-observe.js#newcode343
src/object-observe.js:343: try {
On 2013/05/09 15:39:50, rafaelw wrote:
Andreas,
Is crankshaft going to ignore this try/finally? If so, how should be
approach
this?
We need to be sure to clean-up in the case that the the changeFn
throws?
Yes, Crankshaft won't be able to handle it, hence will never optimise
this function. I'm not sure how performance-critical it actually is,
though. In any case, you can always pull out the 'try' into an auxiliary
function.
https://codereview.chromium.org/14779011/diff/13001/src/object-observe.js
File src/object-observe.js (right):
https://codereview.chromium.org/14779011/diff/13001/src/object-observe.js#newcode86
src/object-observe.js:86: function createObserver(callback, accept) {
Nit: can we capitalise functions consistently?
https://codereview.chromium.org/14779011/diff/13001/src/object-observe.js#newcode124
src/object-observe.js:124: for (; i < from.length; i++, j++) {
I think you can simplify the logic of this loop by not doing j++ here.
https://codereview.chromium.org/14779011/diff/13001/src/object-observe.js#newcode136
src/object-observe.js:136: function moveObserversWhichAre(conditionFn,
from, to, objectInfo) {
Aren't all observers? :)
Can we perhaps call this RepartitionObservers or something like that?
https://codereview.chromium.org/14779011/diff/13001/src/object-observe.js#newcode152
src/object-observe.js:152: if
(IS_UNDEFINED(objectInfo.performing[type]))
How about writing this as:
objectInfo.performing[type] = (objectInfo.performing[type] || 0) + 1
https://codereview.chromium.org/14779011/
--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/groups/opt_out.
