The newest version of YARR supports some optimizations that are important for benchmarks (see bug 605385, for instance), and it'd be good to try and keep pace with upstream anyway. This looks like a messy process; it'd be nice to use an update.sh style for this import to cut down on hacks later. I don't know if there are useful bits in the (well-separated!) patch series from bug 625600 that we could re-use.

What's update.sh? Bug 635600 comment 32 documents my original patch series and the process I had imagined for the next time around, but you may be able to come with something better. Note that a few other Yarr bugfixes have landed since then. Another idea I had was to start with the rev of import-yarr-rev-86639.diff (call it Y1), apply the Yarr updates to that (call this Y2), and then merge Y2 and tip to form Y3. diff(Y3, Y2) would be the adapt-Yarr-new-to-SpiderMonkey diff. The only downside is that this loses the separation of the adapt-Yarr into memory management, bugfixes, etc. I think you could do a similar thing patch by patch (instead of merging to tip, merge to each applied adapt-Yarr patch) in order to update the individual adaptation patches.

(In reply to David Mandelin from comment #1) > What's update.sh? http://mxr.mozilla.org/mozilla-central/find?string=/update.sh A lot of other "import third-party code" directories in the tree have this script, update.sh, that you run like so: cd /path/to/mozilla/directory && sh update.sh /path/to/directory/to/import and the appropriate files get copied over, along with applying a patch series not unlike the patch series you posted in bug 625600 comment 32. > Bug 635600 comment 32 documents my original patch series and the process I > had imagined for the next time around, but you may be able to come with > something better. Note that a few other Yarr bugfixes have landed since then. I don't have any good ideas at the moment, especially as a new version of Yarr probably requires a new version of the assembler, which I assume has had improvements/bugfixes made along the way on our side. So that's double the work right there. Whether an assembler import is needed too, it gets ugly, and there are differences that need to be fixed up (mozilla::CheckedInt vs. WTF::Checked APIs, etc.). I have the rest of this week to work on "performance problems", so I will stare at the code on Thursday and Friday and see if I can't get some semblance of sanity to come out of the merge.

(In reply to Nathan Froyd (:froydnj) from comment #2) > I don't have any good ideas at the moment, especially as a new version of > Yarr probably requires a new version of the assembler, which I assume has > had improvements/bugfixes made along the way on our side. So that's double > the work right there. Whether an assembler import is needed too, it gets > ugly, and there are differences that need to be fixed up > (mozilla::CheckedInt vs. WTF::Checked APIs, etc.). Last time around, I did spot updates to the assembler. There weren't too many of them, but it still took some time. I think it was mostly the checked stuff.

(In reply to David Mandelin from comment #3) > Last time around, I did spot updates to the assembler. There weren't too > many of them, but it still took some time. I think it was mostly the checked > stuff. Having looked at this today, I can confirm that, yes, updates to the assembler are required. In addition, it looks like even what we imported and the assembler at the webkit svn revision are somwhat different, which is quite puzzling to me (and also the source of a number of conflicts). The approach I'm going with for now is simply to import each WebKit revision that touches yarr and the assembler, attempting to keep things compiling along the way.

CheckedInt has now moved to mfbt/ on inbound. Note that WebKit is already using CheckedInt and I've pinged a couple people about syncing their copies. (I infer from above discussion that Yarr is something from WebKit but I have no clue).

We need somebody to do this. They also landed 8bit chars recently, not sure what role this is going play here.

Dave, is there someone that can be assigned to this? There are way too many non-trivial changes at this point for this to be in my comfort zone to try.

This patch imports Yarr and some assorted WTF files from WebKit revision 130234, wholesale replacing any files from their prevision revision (86639). It does not contain any Mozilla modifications, so it does not build.

Most of the cherry-picked changes in the diff of our current tree against the old Yarr revision, were already upstream. So most of the changes were around minor differences: (1) Changing CPU()/COMPILER() macros. (2) JSLinearString/JSC string differences (like our lack of 8-bit support). (3) Things our build system will whine about (extra semicolons, commas). (4) new -> js_new (5) propagating errors in a few places This passes jit-tests (didn't try jstests yet). Tomorrow I'll take another pass, clean up some of the #if 0s and unnecessary changes, and do a tbpl run. A neat feature Yarr has now is that it will can compile two versions of a regex: one for matching only and one for the full output grabbing, which I guess can be used to optimize Regex.test over Regexp.match? I'm not sure yet. I'll file a separate bug for that when this is done, as it could be a good performance win.

This one passes tests. Let's see how it goes: https://tbpl.mozilla.org/?tree=Try&rev=2cd19d1f321f

fix android build, and fix an assert firing on tbpl. windows builds are still busted. https://tbpl.mozilla.org/?tree=Try&rev=abae66c0a134

Another go, this should fix Windows builds. There's also some M-other failures from the last push that I can't reproduce locally, will see if that happens again. https://tbpl.mozilla.org/?tree=Try&rev=db5484d09d3e

Comment on attachment 669363 [details] [diff] [review] part 2, mozilla adaptations Review of attachment 669363 [details] [diff] [review]: ----------------------------------------------------------------- Patch looks good. But I have a question: I see that you folded our local bug fixes (PPC, character classes, alternating groups optimization) into this patch. Do you have a separate copy for them? I think it would be useful to maintain them separately so that we can do things like easily drop one of the bug fixes if WebKit fixes it upstream.

(In reply to David Mandelin [:dmandelin] from comment #13) > Comment on attachment 669363 [details] [diff] [review] > part 2, mozilla adaptations > > Review of attachment 669363 [details] [diff] [review]: > ----------------------------------------------------------------- > > Patch looks good. But I have a question: I see that you folded our local bug > fixes (PPC, character classes, alternating groups optimization) into this > patch. Do you have a separate copy for them? I think it would be useful to > maintain them separately so that we can do things like easily drop one of > the bug fixes if WebKit fixes it upstream. I can extricate some of the syntactic ones and the alternating groups optimization, but many of the changes are just fixing things that don't compile in our build system (like CPU(X86) -> WTF_CPU_X86) which we probably couldn't upstream.

Potentially upstreamable syntactic tweaks.

Upstreamable patch to handle errors in the offset setup code.

Upstreamable patch to make consumeNumber() fallible.

I separated this out into a new patch, but it looks like WebKit explicitly wanted different functionality.

These changes, I think, are mostly non-upstreamable (let me know if I missed any or if we can bridge some more).

Finally got time to look into the win64 problems. New try run: https://tbpl.mozilla.org/?tree=Try&rev=c3fcb4ffc474

VS2008SP1 (VC9) I'm getting lots of "Cannot open include file: 'stdint.h': No such file or directory" c:\t1\hg\comm-central\mozilla\js\src\yarr\CheckedArithmetic.h(34) : fatal error C1083: Cannot open include file: 'stdint.h': No such file or directory