Bug 204760 - [JSC] Remove BytecodeCacheVersion.h
Summary: [JSC] Remove BytecodeCacheVersion.h
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Tadeu Zagallo
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2019-12-02 11:33 PST by Tadeu Zagallo
Modified: 2019-12-02 15:24 PST (History)
11 users (show)

See Also:


Attachments
Patch (4.86 KB, patch)
2019-12-02 13:21 PST, Tadeu Zagallo
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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.