13 ms·
On a development note, is it normal to drop large features like this a single commit?
by Game_Ender 15y ago
On a development note, is it normal to drop large features like this a single commit?
- ootachi 15y agoIt's Apple's (and Google's) modus operandi. They work in secret, and then they make huge code drops every so often.
- dchest 15y agohttps://bugs.webkit.org/show_bug.cgi?id=75812 https://bugs.webkit.org/show_bug.cgi?id=75812
- olliej 15y agoIf you actually look at the commit logs for javascriptcore you see all development goes on in public, all patches follow the webkit review rules (you can't do an unreviewed code dump, and large code dumps aren't pleasant when you need reviews). JSC is developed in the open, everyone can see the work we're doing, and the progress we're making, as we're making it. If you had actually followed the link you would have seen a change log pointing to a bug on bugs.webkit.org that shows the huge amount of (public) work that preceded actually landing this https://bugs.webkit.org/show_bug.cgi?id=75812 https://bugs.webkit.org/show_bug.cgi?id=75812 . Additionally there was substantial refactoring in multiple commits leading up to the final landing of LLInt in order to reduce the patch size as much as possible. The reason the LLInt landed as a "code dump" is because that is the minimal size possible - you can't land part of an execution engine, as by definition part of an engine is not sufficient to pass tests. As it is, this is only 32 bit for the simple reason that 32-bit x86 is the easiest way to stress register allocation, our value encoding requires two 32bit registers per value and 32bit x86 has very few registers. TLDR; You can't really do massive codedumps in the webkit repository, the commit rules make it very hard. Alas some patches are intrinsically large though as a partial patch won't be sufficient to pass tests, but even then we don't like them.
- ootachi 15y agoMy apologies then. Google does do that with V8, but your practices sound great.
- olliej 15y agoTricky question: We try to keep all patches small, as the larger a patch is the longer it will take to get reviewed, and the longer the review takes, and the more update-review-update-review... cycles are required. There are some things (like this patch) where the core change (in this case a new execution engine) can't be landed in pieces (either you have a complete working execution engine that will pass all tests, and won't regress performance, or you don't). But even in these cases we try to reduce the size and complexity of the core patch. If you look at the recent commits to JSC you'll see a bunch of refactoring landing prior to LLInt that got the non-LLInt specific code into the tree in advance of LLInt itself. As it is the initial commit only has support for our 32bit encoding, and doesn't support all the backends that the JSC JIT supports. The basic reason for this is that the static assembler does need to do some register allocation and the easy way to ensure that it's correct is to through it at 32 bit x86. Our value representation requires 2 registers on 32 bit platforms, and x86-32 has very few GPRs we can use, so it acts as an excellent stress test vs. any other supported platform.