Bug 31246 - DateMath.cpp needs to be split into JSC and WTF portions.
: DateMath.cpp needs to be split into JSC and WTF portions.
Status: RESOLVED WORKSFORME
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore
: 528+ (Nightly build)
: All All
: P2 Normal
Assigned To: Nobody
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2009-11-08 18:24 PST by David Levin
Modified: 2012-01-09 16:24 PST (History)
3 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description David Levin 2009-11-08 18:24:39 PST
The JSC portion should probably move into JavaScriptCore/runtime (from wtf).
Comment 1 David Levin 2009-11-08 18:26:26 PST
When the split is done, the USE jsc defines in JavaScriptCore/config.h and JavaScriptGlue/config.h should be removed.
Comment 2 David Levin 2009-11-08 18:34:33 PST
I divided up the functionality in http://trac.webkit.org/changeset/50633 but left in one file for the moment so that I could get in the chromium build fix more quickly.
Comment 3 Geoffrey Garen 2009-11-09 11:59:18 PST
I don't think it's a good idea to split DateMath.cpp into JSC and WTF portions.

For one thing, I just did the opposite in bug 31197. I wish you hadn't reverted my work without talking to me or even mentioning why. The comment I made in that ChangeLog was:

"Moved most of the code in DateMath.* into the JSC namespace. The code needed to move so it could naturally interact with ExecState and JSGlobalData to support caching. Logically, it seemed right to move it, too, since this code is not really as low-level as the WTF namespace might imply -- it implements a set of date parsing and conversion quirks that are finely tuned to the JavaScript language."

Do you disagree with this comment? If so, why?
Comment 4 David Levin 2009-11-09 13:39:22 PST
Well, this totally broke the chromium port.

I figured this must have been an oversight because I know that folks in WK take great care not to break any of the ports as evidenced by subsequent fixes for qt, etc.

So I was trying to fix one of the ports as quickly as possible which is why I did this quickly without discussing it, *but* I created this bug and cc'ed you so I wasn't trying to hide this at all.

In general as I read your patch it was about using exec to have a perf increase, and I kept that part of it. Anyway, I don't know how well we can discuss this in a bug but I'm happy to work with you to figure something out.  I'm in irc as I usually am (dave_levin).
Comment 5 Geoffrey Garen 2009-11-11 13:09:16 PST
It's harder to keep Chromium building that other ports, because it uses a substantially different configuration than the default WebKit configuration, and much of it is not in the WebKit repository.

It also hasn't been a priority for me because Chromium developers have told me not to make it a priority -- the implication being that we shouldn't have to slow down mainline development of WebKit to support v8. Please let me know if your position on this has changed.

I'm not terribly familiar with the version of the Chromium port that uses v8, but here are some suggestions for how you could move forward:

- Change your version of WebCore to use the v8 date parser instead of the JavaScriptCore date parser.

- Build just enough of JavaScriptCore into your port so that you can use the JavaScriptCore date parser.

- Build a separate date parser for HTTP, and have WebCore use that instead. (This is the best solution, but also the most time-consuming.)

Which of these solutions sounds best to you?
Comment 6 David Levin 2009-11-11 15:56:27 PST
(In reply to comment #5)

I don't think that going back and forth in this bug is the best way to resolve this. I tried to ping you in irc yesterday but we missed each other.

> It also hasn't been a priority for me because Chromium developers have told me
> not to make it a priority -- the implication being that we shouldn't have to
> slow down mainline development of WebKit to support v8. Please let me know if
> your position on this has changed.

Well, if you can keep it building, then great.

If not, then a chromium person needs to go around and clean up as best as possible after these changes to keep chromium building in some way.  Just like any other port we make it a priority to keep building all the time.