-
Notifications
You must be signed in to change notification settings - Fork 75
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
Benchmark Onyx #65
Comments
You don't have to patch timer logic inside Onyx code You can decorates existing methods like this: import Onyx from './lib/Onyx';
import withOnyx from './lib/withOnyx';
if (process.env.ONYX_INSTRUMENTATION == 'true') {
const decorator = require('./lib/benchmarkDecorator');
decorator.decorate(Onyx);
}
export default Onyx;
export {withOnyx}; And the decorator would do something like this function decorate(Onyx) {
const promiseReturningMethods = ['merge', 'set', 'get', ...];
promiseReturningMethods.forEach(methodName => {
const original = Onyx[methodName];
const decorated = (...args) => {
const logId = Log.timeStart(methodName, args);
return original.apply(Onyx, args)
.finally(() => Log.timeEnd(logId))
};
Onyx[methodName] = decorated;
});
} If Onyx had a prototype (was a class) you would've been able to also track internal usage by this decorating pattern Something similar can be done for methods like This could be further extended to track call counts and memory usage.
|
Oh, this is cool! Some questions to help me understand it:
Basically, I don't think there's any doubt that adding a RAM cache to Onyx would improve read performance. The only question is "by how much" and "can we afford it?" Because my understanding (but please correct me if I'm wrong) is that we regularly run out of RAM even without this cache, so in practice we just don't have enough RAM to spare. So it doesn't really matter how well a cache might work: we just don't have enough RAM to use it. |
Incidentally, I'm looking into the implementation of AsyncStorage.get() on Android, and it looks pretty inefficient -- so far as I can tell, it isn't using prepared statements, so it's compiling a totally distinct SQL statement for every single get. This feels like we could pretty easily optimize by:
I think the benchmarking I'd like to see would be of AsyncStorage today, and AsyncStorage with the above optimizations. I bet we could significantly improve read/write performance of AsyncStorage itself on Android, and then contribute this upstream so everyone using ReactNative can benefit. |
Additionally, we should look at what settings are used when opening the sqlite database itself. Specifically, I'd suggest Additionally, we should look into enabling memory mapping of SQLite, as that will likely provide another substantial performance boost. |
The nice thing about memory mapping SQLite is it provides us a RAM cache for free, using the virtual memory subsystem of Linux. So we wouldn't need to actually build a RAM cache into Onyx, because recently used sqlite pages would already be in RAM. |
Yes, but it's not an on demand switch. You either make a build with this enabled or not. This way no unused benchmark code would end up being part of the production bundle
How changes to Onyx affect performance.
I guess this will be one more thing ending up in the "unproven solutions" bucket, but you are actually using more RAM because Onyx is constantly reading from disk. I've tried to explain that on the slack thread. I don't want to delve into details, but here's a short list:
If e.cash is running out of RAM I'm 99% sure it's caused by a memory leak, and not because the actual data is taking too much RAM How much do you expect memory usage to grow after app initialization e.g. I don't think opening a chat or a few at once would ever go beyond 50-60MB of that initial memory footprint. And then as you switch between chats resources should be freed and allocated again. The Chat underlying implementation is a VirtualList - resources that aren't visible are freed from memory - they would only take as much as their text content represented in Onyx. |
Closing this as benchmark logic was implemented in #63 |
Problem
Measure the performance of Onyx
Execution time
connect
and resolving data to the caller. Same for other methods likemerge
,set
,get
...Method call counts and keys used
connect
called often to retrieve the same key?Onyx memory footprint
Solution
TBD
The text was updated successfully, but these errors were encountered: