Bug 204760

Summary: [JSC] Remove BytecodeCacheVersion.h
Product: WebKit Reporter: Tadeu Zagallo <tzagallo>
Component: JavaScriptCoreAssignee: Tadeu Zagallo <tzagallo>
Status: RESOLVED FIXED    
Severity: Normal CC: annulen, commit-queue, ews-watchlist, gyuyoung.kim, keith_miller, mark.lam, msaboff, ryuan.choi, saam, sergio, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch none

Description Tadeu Zagallo 2019-12-02 11:33:10 PST
...
Comment 1 Tadeu Zagallo 2019-12-02 13:21:46 PST
Created attachment 384651 [details]
Patch
Comment 2 Mark Lam 2019-12-02 13:32:01 PST
Comment on attachment 384651 [details]
Patch

Are we guaranteed that the 2 places that "call" jscBytecodeCacheVersion() will produce the same value?  Is there any chance that the time stamp changed by a small fraction between those 2?
Comment 3 Tadeu Zagallo 2019-12-02 13:49:30 PST
(In reply to Mark Lam from comment #2)
> Comment on attachment 384651 [details]
> Patch
> 
> Are we guaranteed that the 2 places that "call" jscBytecodeCacheVersion()
> will produce the same value?  Is there any chance that the time stamp
> changed by a small fraction between those 2?

There are two calls to the function, but since __TIMESTAMP__ is a preprocessor macro[1] it will be replaced by a constant string before the parser runs, so it should always produce the same value.

[1]: https://gcc.gnu.org/onlinedocs/cpp/Common-Predefined-Macros.html
Comment 4 Mark Lam 2019-12-02 13:52:17 PST
Comment on attachment 384651 [details]
Patch

r=me
Comment 5 WebKit Commit Bot 2019-12-02 14:45:32 PST
Comment on attachment 384651 [details]
Patch

Clearing flags on attachment: 384651

Committed r253010: <https://trac.webkit.org/changeset/253010>
Comment 6 WebKit Commit Bot 2019-12-02 14:45:34 PST
All reviewed patches have been landed.  Closing bug.
Comment 7 Radar WebKit Bug Importer 2019-12-02 14:46:28 PST
<rdar://problem/57571599>
Comment 8 Saam Barati 2019-12-02 15:21:13 PST
Why is this correct? What about collisions?
Comment 9 Saam Barati 2019-12-02 15:23:48 PST
(In reply to Saam Barati from comment #8)
> Why is this correct? What about collisions?

This patch looks wrong. Before, we had a monotonically increasing number. Now, we’re using a hash function which is bound to have collisions.
Comment 10 Mark Lam 2019-12-02 15:24:49 PST
(In reply to Saam Barati from comment #9)
> (In reply to Saam Barati from comment #8)
> > Why is this correct? What about collisions?
> 
> This patch looks wrong. Before, we had a monotonically increasing number.
> Now, we’re using a hash function which is bound to have collisions.

Hmmm, I agree.  I should have thought of that.