Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(34)

Issue 9307083: Implement caching scheme for Date fields. (Closed)

Created:
8 years, 10 months ago by rossberg
Modified:
8 years, 9 months ago
Reviewers:
ulan
CC:
v8-dev
Visibility:
Public.

Description

Implement caching scheme for Date fields. This is based on https://chromiumcodereview.appspot.com/9117034/ Doesn't have much impact on its own, but is the basis for Ulan's CL https://chromiumcodereview.appspot.com/9117034/, which moves the logic to C++. R=ulan@chromium.org BUG= TEST= Committed: https://code.google.com/p/v8/source/detail?r=10982

Patch Set 1 #

Patch Set 2 : Fixed register allocation (and rebased). #

Total comments: 4

Patch Set 3 : Addressed Ulan's comments. #

Patch Set 4 : Rebased to HEAD. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+165 lines, -107 lines) Patch
M src/api.cc View 1 2 3 1 chunk +1 line, -1 line 0 comments Download
M src/arm/lithium-arm.cc View 1 1 chunk +2 lines, -2 lines 0 comments Download
M src/date.js View 1 2 25 chunks +77 lines, -41 lines 0 comments Download
M src/ia32/lithium-ia32.cc View 1 1 chunk +2 lines, -2 lines 0 comments Download
M src/macros.py View 1 chunk +18 lines, -14 lines 0 comments Download
M src/objects.h View 1 4 chunks +10 lines, -6 lines 0 comments Download
M src/objects-debug.cc View 1 1 chunk +34 lines, -30 lines 0 comments Download
M src/objects-inl.h View 1 2 chunks +7 lines, -1 line 0 comments Download
M src/objects-printer.cc View 1 2 2 chunks +12 lines, -8 lines 0 comments Download
M src/x64/lithium-x64.cc View 1 1 chunk +2 lines, -2 lines 0 comments Download

Messages

Total messages: 3 (0 generated)
rossberg
8 years, 10 months ago (2012-02-03 14:33:42 UTC) #1
ulan
LGTM if comments are addressed. http://codereview.chromium.org/9307083/diff/2001/src/date.js File src/date.js (right): http://codereview.chromium.org/9307083/diff/2001/src/date.js#newcode674 src/date.js:674: // SET_DATE_LOCAL(this, t); Commented ...
8 years, 9 months ago (2012-03-08 08:12:42 UTC) #2
rossberg
8 years, 9 months ago (2012-03-08 09:53:41 UTC) #3
https://chromiumcodereview.appspot.com/9307083/diff/2001/src/date.js
File src/date.js (right):

https://chromiumcodereview.appspot.com/9307083/diff/2001/src/date.js#newcode674
src/date.js:674: // SET_DATE_LOCAL(this, t);
On 2012/03/08 08:12:42, ulan wrote:
> Commented code here and below.

Done.

https://chromiumcodereview.appspot.com/9307083/diff/2001/src/objects-printer.cc
File src/objects-printer.cc (right):

https://chromiumcodereview.appspot.com/9307083/diff/2001/src/objects-printer....
src/objects-printer.cc:679: weekdays[weekday()->IsSmi()?
Smi::cast(weekday())->value() + 1 : -1],
On 2012/03/08 08:12:42, ulan wrote:
> if weekday is not a Smi, the index should be 0 not -1

Done.

Powered by Google App Engine
This is Rietveld 408576698