-
Notifications
You must be signed in to change notification settings - Fork 630
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Date 800x slower than JSC #930
Comments
Thank you for reporting this. It looks like an actual problem and is likely caused by us not caching the timezone information. As an experiment, we changed the system timezone while the JS process was running to see whether it would invalidate the timezone. Hermes displayed the correct new timezone, while JSC and v8 displayed the original one. It is not entirely clear which behavior is preferable. Could you try a similar experiment in your mobile app? |
I can confirm this, I previously used spacetime js to simplify date comparisons in my expo managed app. With the newest release of expo 48 we transitioned to hermes engine and had to remove all spactime js uses because just 150 items slowed down the app to a standstill for several seconds whilst running date comparisons. This is now our code: And this is also slower than it was using jsc, by a large margin. |
@tmikov this is a major problem for anything which relies on dates to generate unique identifiers or localized translations.. Both iOS and android have mechanisms to notify the app when there is a timezone change. IMO, caching the timeszone info and then updating that info only when there is an actual timezone change could be a potential solution here. |
We have exactly the same issue. Because of it we cannot switch to Hermes engine unfortunately. I run this function in a sorting loop (~500 items in a list)
Here is a preview: And then I tried to use isSame function form dayjs and it took ~15500ms!!! to complete sorting |
We agree that this needs to be fixed to match the behavior of the other engines. Unfortunately we don't have concrete plans on when exactly we can work on the fix yet. Meanwhile a community contribution would be welcome. |
Thank you for the quick response @tmikov! |
The "solution" in our case is indeed not using Hermes atm. |
@tmikov The simplest most straight forward solution here would be to expose an API from |
@rpopovici are there APIs for updating the cached timezone information in JSC? In other words, if a RN app is running in JSC and a system timezone change occurs, how can you force it to refresh its cache? If there is no such API, how can the app handle timezone changes at all? |
@tmikov if relying on timezone change events is a deviation from the standard behaviour, then trying to debounce the timezone info every second could be an acceptable compromise solution which could alleviate the perf bottleneck and is better than JSC or v8 since these two won't update the timezone data at all. Also the timezone debouncing solution can be easily exposed through a hermes runtime config flag and then you can keep both behaviours and let the user decide if they need super accurate timezone info or they can settle for something less accurate but more performant |
Listening for
Don't forget to remove the subscription for
This should allow you to update the cached timezone data only when |
@rpopovici this indeed looks like an attractive solution. What is the thread safety of the callback, i.e. in what thread will it be invoked? I suspect the safest thing to do here is to set a per-runtime atomic flag and check it later. |
@tmikov When
|
Hmm, unfortunately Hermes doesn't own a thread or an NSOperationQueue. It executes in the context of an arbitrary thread provided by an integrator. Adding functionality like this is never as simple as it appears. So, let's clarify the scope of the problem. Other engines like v8 and JSC cache the timezone permanently with no ability to update it, even if the system timezone changes. Apparently RN developers find that behavior acceptable. It seems like we can duplicate that behavior in Hermes. Is that a reasonable conclusion? |
@tmikov can't speak for others but it seems ok for our case. |
Any updates on this? Still blocking us from switching to hermes |
I think this seems like the most likely culprit destroying my app performance with hermes. We have a fair amount of date logic since part of the app is a todo list. The app runs 4-6x slower on hermes than on JSC or v8. Hopefully this can get bumped up in priority. It seems like it's impacting a fair number of projects. There are probably many projects using hermes by default now that are suffering poor performance because of this. Perhaps some basic performance tests should be added to the hermes test suite to hopefully catch app breaking performance regressions like this before they get out to production apps. |
@evelant do you have a repro of your problem or is this just a guess? How many date calculation per second is your app performing and what kind of calculation? What does "runs 4-6x slower" mean? Is the update rate 6 times slower? Is your app performing non-stop date calculations in a loop? |
Unfortunately I don't have a repro, it's just a guess. By 4-6x slower I mean that user-perceived responsiveness is 4-6x slower. Everything in the app takes 4-6x longer when running on hermes. My best guesses so far at likely culprits are:
Sorry I don't have more than a guess at the moment. I don't currently have the time to really pick apart the app to narrow it down. We're just going to run on JSC or v8 until there's more room to investigate. |
@tmikov you can't control how many calls / sec are out there. Most modern apps use translation and text formatting libraries which rely heavily on This is why |
@rpopovici Agreed. For example our app by default groups tasks by day for a calendar view. If someone has a couple hundred tasks we're tripping over exactly the issue @DePavlenko described above where a simple |
@evelant help me understand, why do you need to perform local time calculations for hundreds of tasks? Ordinarily, local time would only be used for display, and all logic would use UTC? |
@tmikov Most of the logic uses UTC, but we also use libraries like date-fns that might or might not use Date instances under the hood. As others have said however, it doesn't really matter -- it's not an app problem since the app is fine on JSC and v8, it's a hermes problem. If using particular date libraries or accidentally doing some date calculations using Date objects instead of UTC timestamps can cause such a massive slowdown that's a serious bug in hermes, you can't expect app/library developers to work around that just for hermes. |
Can someone please provide a real example (not a synthetic one) demonstrating the problem? The already synthetic examples are illustrative, but for real optimization work we need examples of actual useful code that needs its performance improved. |
BTW, I want to note that the JSC performance in iOS simulator is not representative, because it uses JIT, while JIT is disabled on real devices because of Apple policies. So, it is pointless to compare JSC in simulator to Hermes in simulator. |
@rpopovici the latest perf results are unexpected. So, normal CPU-bound code executes with the same performance in the simulator and on the host, but localized date() operations in the same engine are much slower in the simulator. This is so surprising that we need to confirm it. But if it is true, it must mean that for some reason standard libraries on iOS throttle the implementation of functions like |
@rrebase I was able to simplify your example to the following: const t0 = Date.now();
for (let i = 0; i < 500; i++) {
new Date('2021-09-15T18:30:00.000Z').toLocaleTimeString([], { hour: '2-digit', minute: '2-digit' });
}
const t1 = Date.now();
(typeof print !== "undefined" ? print: console.log)(`Call took ${t1 - t0} milliseconds.`); Running this simplified test, v8 takes about 30 ms, while Hermes takes 230 ms. This test exercises only the performance of So, I modified the example in the following way: const t0 = Date.now();
const df = new Intl.DateTimeFormat('en-US', { hour: '2-digit', minute: '2-digit' });
for (let i = 0; i < 500; i++) {
df.format(new Date('2021-09-15T18:30:00.000Z'));
}
const t1 = Date.now();
(typeof print !== "undefined" ? print: console.log)(`Call took ${t1 - t0} milliseconds.`); With this change both v8 and Hermes take less than 15ms. My conclusion is that |
FYI, we are working on this. Asking questions and commenting here doesn't mean we are waiting. |
@tmikov I tested again both
|
@rpopovici oh great, this is becoming more and more bizarre (to be fair most interesting real world problems are). So, it seems the throttling is only on iOS Simulator. and performance is much better on a physical device? How bad is the 227ms number that you are getting from device? Can you compare it to JSC? |
@tmikov I edited the previous post #930 (comment) |
@rpopovici your latest results make it look like Hermes is in about the same performance ballpark as JSC when using Spacetime on a physical iOS device. Is my interpretation of your numbers correct? It seems that we have proven two things:
Perhaps we should focus on the Dayjs benchmark which was posted earlier and where I was able to reproduce the slowness locally. |
@tmikov you are correct. |
Hello 👋🏻 @tmikov, following our discussion during React Native Europe, I'd love to keep this moving. Can you clarify if:
|
@Titozzz thanks for posting here! Here are my initial comments:
If this could be a community contribution, we should discuss the approach in detail ahead of time. As I mentioned before, the main problem here is caching. Caching of the time range with a fixed offset surrounding a given timestamp. So for example if we are working with a timestamp on Aug 15th 2010 and DST starts on Sep 15th and ends June 1st, then the range is from June 1st 2010 to Sep 15th 2010 (with hours and minutes, of course). Once we have that, we can convert times in that range to UTC and back very efficiently. Of course we should cache multiple of these for multiple timezones. I am simplifying a bit, but I hope this gives enough of initial context. Plus, keep in mind that I am not an expert on this, not even a little bit! Additionally there should be probably be another layer of cache for a certain number of timestamps, since it is likely that the same timestamp will be operated on multiple times, for example when sorting. This is an optional improvement that can be added later. So, there seem to be two approaches for achieving this:
I hope this gives you an idea of what is necessary and why we haven't fixed it yet... |
I'm pretty sure iOS does not provide an API for time zone information, only supporting queries for dates. This is not per se disqualifying of method 1: I would be surprised if allocation-free use of the iOS API was not performant, but it does prevent abstracting the platform away at the level of time zones. Method 2 is to my knowledge the typical choice. Risk of incompatibility from partial repacking can be mitigated by preferring dropping time zones to packing partial data for a time zone (and in general I don't think strong guarantees are or should be expected with regards to localized times). I did notice that tzdb is distributed in plaintext with substantial commentary, I'm wondering if after compilation it's more viable to pack? |
I ran into the same performance issues when dealing with dates on the iOS simulator and investigated a bit. tzset() is the culpritI've reproduced the issue by making a native app that calls Hermes' #import "DateUtil.h"
double getTime(void) {
struct timeval tv;
gettimeofday(&tv, NULL);
return (double)(tv.tv_sec) * 1000 + (double)(tv.tv_usec) / 1000;
}
int main(int argc, char * argv[]) {
double t1 = getTime();
for (int i = 0; i < 100000; i++) {
localTime(getTime());
}
double t2 = getTime();
printf("Duration: %lf\n", t2 - t1);
} Results:
On cachingJavascript Core implements caching of local time offsets here. The cache is invalidated if consecutive dates are separated by more than one month from each other. I've benchmarked the impact on cache using this code, executed on an iOS simulator from a React Native app with JSC enabled: const iters = 1000000;
const msPerHour = 60 * 60 * 1000;
const msPer2Months = 2 * 30 * 24 * msPerHour;
const t1 = performance.now();
for (let i = 0; i < iters; i++) {
// Cache will be effective as the dates are separated by just one hour
const date = new Date(1696830297000 + i * msPerHour);
date.getHours();
}
const t2 = performance.now();
for (let i = 0; i < iters; i++) {
// Cache will be invalidated each time as the dates are separated by 2 months
const date = new Date(1696830297000 + i * msPer2Months);
date.getHours();
}
const t3 = performance.now();
console.log('Benchmark results:', t2 - t1, t3 - t2); Results:
Conclusion: |
@simontreny The funny thing is that when I run my apps with JSC with a debugger, all performance issues go away... I do have a simple example of a situation where I setup a scheduler for a week overview without any library for date handeling. It is setup in expo though.. |
@TiStyle I refactored all of my code to avoid Date instances (except for display in the UI). I just use plain numbers and do simple math when I need to add/subtract/calculate things. Use |
@evelant I was hoping to avoid such things, but it seems there is no other solution but for now(). Haha, get it?.. Thank you for your advice, I'll just have to start this painstakingly task. |
I wanted to share that we are also facing this issue in our React Native app. We have an infinite list of messages, each of which render a date, which must be calculated by the difference between sendTime and now. To "solve" this, we can eagerly render the list, then defer the date calculations inside InteractionManager. This greatly sped up the initial paint of the list. /**
* DateTime calculation are currently very slow in Hermes https://github.com/facebook/hermes/issues/930
* This means each MessageTime would add about 20ms to the list item render.
*
* To 'resolve' this, we start with empty string, then wrap the formatFunction in InteractionManager,
* so that it does block the first paint. This tradeoff is that the timestamp loads slightly after the message,
* but the message list renders in 1/3rd the time.
*
* This optimisation can be removed when the above issue is solved.
*
*/
export const MessageTime = ({ timestamp, prefix }: Props) => {
const [text, setText] = useState('');
useEffect(() => {
const calculateStamp = () =>
InteractionManager.runAfterInteractions(() => {
setText(formatTimestamp(new Date(), timestamp));
});
calculateStamp();
const interval = setInterval(() => {
calculateStamp();
}, oneMinute);
return () => clearInterval(interval);
}, [timestamp]);
return (
<View style={styles.container}>
<Text muted fontSize={'sm'} numberOfLines={1}>
{prefix}
{text}
</Text>
</View>
);
};
const styles = StyleSheet.create({
container: {
display: 'flex',
flexDirection: 'row',
flexShrink: 1,
},
}); |
In our case DayJS was the problem, so I made some quick performance improvments and it now runs 1000x faster on iOS simulator: NOTE: See @km-tr 's comment below to address using dayjs().set(...) as my solution does not account for mutability. diff --git a/node_modules/dayjs/esm/index.js b/node_modules/dayjs/esm/index.js
index a82986b..1fb8898 100644
--- a/node_modules/dayjs/esm/index.js
+++ b/node_modules/dayjs/esm/index.js
@@ -117,17 +117,101 @@ var Dayjs = /*#__PURE__*/function () {
_proto.init = function init() {
var $d = this.$d;
- this.$y = $d.getFullYear();
- this.$M = $d.getMonth();
- this.$D = $d.getDate();
- this.$W = $d.getDay();
- this.$H = $d.getHours();
- this.$m = $d.getMinutes();
- this.$s = $d.getSeconds();
- this.$ms = $d.getMilliseconds();
+ // this.$y = $d.getFullYear();
+ // this.$M = $d.getMonth();
+ // this.$D = $d.getDate();
+ // this.$W = $d.getDay();
+ // this.$H = $d.getHours();
+ // this.$m = $d.getMinutes();
+ // this.$s = $d.getSeconds();
+ // this.$ms = $d.getMilliseconds();
} // eslint-disable-next-line class-methods-use-this
;
+ // Avoid running native Date functions like getFullYear until needed because these
+ // functions are super slow in Hermes. Then run once and cache since Dayjs is immutable.
+
+ Object.defineProperty(Dayjs.prototype, '$y', {
+ get() {
+ if (!this.cached_y) {
+ this.cached_y = this.$d.getFullYear();
+ }
+ return this.cached_y;
+ }
+ });
+
+ Object.defineProperty(Dayjs.prototype, '$M', {
+ get() {
+ if (!this.cached_M) {
+ this.cached_M = this.$d.getMonth();
+ }
+ return this.cached_M;
+ }
+ });
+
+ Object.defineProperty(Dayjs.prototype, '$D', {
+ get() {
+ if (!this.cached_D) {
+ this.cached_D = this.$d.getDate();
+ }
+ return this.cached_D;
+ }
+ });
+
+ Object.defineProperty(Dayjs.prototype, '$W', {
+ get() {
+ if (!this.cached_W) {
+ this.cached_W = this.$d.getDay();
+ }
+ return this.cached_W;
+ }
+ });
+
+ Object.defineProperty(Dayjs.prototype, '$H', {
+ get() {
+ if (!this.cached_H) {
+ this.cached_H = this.$d.getHours();
+ }
+ return this.cached_H;
+ }
+ });
+
+ Object.defineProperty(Dayjs.prototype, '$m', {
+ get() {
+ if (!this.cached_m) {
+ this.cached_m = this.$d.getMinutes();
+ }
+ return this.cached_m;
+ }
+ });
+
+ Object.defineProperty(Dayjs.prototype, '$s', {
+ get() {
+ if (!this.cached_s) {
+ this.cached_s = this.$d.getSeconds();
+ }
+ return this.cached_s;
+ }
+ });
+
+ Object.defineProperty(Dayjs.prototype, '$ms', {
+ get() {
+ if (!this.cached_ms) {
+ this.cached_ms = this.$d.getMilliseconds();
+ }
+ return this.cached_ms;
+ }
+ });
+
+ Object.defineProperty(Dayjs.prototype, '$timezoneOffset', {
+ get() {
+ if (!this.cached_timezoneOffset) {
+ this.cached_timezoneOffset = this.$d.getTimezoneOffset();
+ }
+ return this.cached_timezoneOffset
+ }
+ });
+
_proto.$utils = function $utils() {
return Utils;
};
@@ -137,15 +221,27 @@ var Dayjs = /*#__PURE__*/function () {
};
_proto.isSame = function isSame(that, units) {
+ if (units === undefined) {
+ return this.toISOString() === dayjs(that).toISOString()
+ }
+
var other = dayjs(that);
return this.startOf(units) <= other && other <= this.endOf(units);
};
_proto.isAfter = function isAfter(that, units) {
+ if (units === undefined) {
+ return this.toISOString() > dayjs(that).toISOString()
+ }
+
return dayjs(that) < this.startOf(units);
};
_proto.isBefore = function isBefore(that, units) {
+ if (units === undefined) {
+ return this.toISOString() < dayjs(that).toISOString()
+ }
+
return this.endOf(units) < dayjs(that);
};
@@ -408,7 +504,7 @@ var Dayjs = /*#__PURE__*/function () {
_proto.utcOffset = function utcOffset() {
// Because a bug at FF24, we're rounding the timezone offset around 15 minutes
// https://github.com/moment/moment/pull/1871
- return -Math.round(this.$d.getTimezoneOffset() / 15) * 15;
+ return -Math.round(this.$timezoneOffset / 15) * 15;
};
_proto.diff = function diff(input, units, _float) { |
I noticed 150ms increase in render time with dayjs, without the date the component rendered in 50ms, converted to Date and it increased to 70ms which is way faster than the 200ms with dayjs. |
@gaberogan For example, today is the 22nd, and when I use However, when I use |
@km-tr I haven't seen this problem, but thanks for mentioning it. If it's a problem with my diff and you find a solution, I'll update the diff so others don't run into the same problem. |
Thank you for checking. _proto.init = function init() {
this.cached_y = undefined;
this.cached_M = undefined;
this.cached_D = undefined;
this.cached_W = undefined;
this.cached_H = undefined;
this.cached_m = undefined;
this.cached_s = undefined;
this.cached_ms = undefined;
this.cached_timezoneOffset = undefined;
} |
@gaberogan Since importing |
We have made some improvements to localTime()/utcTime() performance in Hermes. On a simple benchmark with pure |
@lavenzg Thank you! Do you know when we can expect this fix in the RN? |
HI there, it has been a few months now and we're very eager to find a solution to this. |
The performance improvements were made in 59ea64c, and should be part of RN 0.76. |
Bug Description
Date so slow that it can be freeze app. Impossible to use third party date library(much worse perf, 1800x).
I found that local date is a problem. UTC date still slow but not 800x, just 8x slower than JSC which can be ignored.
The code outputs:
JSC: 0.2605330003425479
Hermes: 166.44708999991417
Hermes UTC: 1.6460870001465082
gradle clean
and confirmed this bug does not occur with JSCHermes version: Bundled version with RN 0.71.3
React Native version (if any): 0.71.3
OS version (if any): iPhone 14 (simulator)
Platform (most likely one of arm64-v8a, armeabi-v7a, x86, x86_64):
Steps To Reproduce
code example:
The Expected Behavior
Fast Date operations.
The text was updated successfully, but these errors were encountered: