Use mmap.PAGESIZE constant as value for first read #505
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
xavfernandez
force-pushed
the
xfernandez/extract_constant
branch
from
9d459f7
to
695f095
Compare
|
With 60000 small files, switching (with a stable timing on multiple runs) to With a strong speedup on second run thanks to the OS filesystem cache. I admit that such numbers depend heavily on:
Therefore I don't think there is an always perfect |
|
It's not good practice to poke around internal constants of code. If there's a better value we should change it. |
I agree it is not ideal. A small I thought about adding a Would this make more sense ? |
|
Thinking a bit more, I'm not sure that this should make a difference at all. If the file is sparse then they'll be handled via the zero page, which doesn't use file cache. Do you have benchmarks? |
|
Well my benchmark is kinda primitive. I create my 60k files by launching 60k times this python program: and I time with a simplified version of what the library performs: Before each run, I wipe the file cache with With a bunch of free memory (~ 26 GB) and a read size of 65535 bytes: With a bunch of free memory (~ 26 GB) and a read size of 4096 bytes: Now with ~ 2 GB of free memory and a read size of 65535 bytes: And finally with ~ 2 GB of free memory and a read size of 4096 bytes: And sure enough, if I constraint the system even more to have less than a 500 MB of free memory, the performance is bad even with 4096: And in the same way, even with 25 GB of available memory, if I push the read size to 650 350 kbytes: |
|
Thanks, looks like we should change to value to 4k.
…On Thu 30 Jan 2020, 16:49 Xavier Fernandez, ***@***.***> wrote:
Well my benchmark is kinda primitive.
I create my 60k files by launching 60k times this python program:
import os
os.environ['prometheus_multiproc_dir'] = 'tmp'
import random
import prometheus_client
METRIC = prometheus_client.Summary(
'some_duration_seconds',
'time spent somewhere',
labelnames=('label1', 'second_label'),
namespace='profile',
subsystem='lot_of_files',
)
METRIC.labels(random.choice(('success', 'error')), f'test{random.randint(0, 10)}').observe(random.random())
METRIC.labels(random.choice(('success', 'error')), f'test{random.randint(0, 10)}').observe(random.random())
METRIC.labels(random.choice(('success', 'error')), f'test{random.randint(0, 10)}').observe(random.random())
and I time with a simplified version of what the library performs:
# This is read_simple.py
import os
import sys
import time
BLOCK_SIZE = int(sys.argv[1])
print('BLOCK_SIZE', BLOCK_SIZE)
start = time.perf_counter()
for filename in os.listdir('tmp'):
if filename.endswith('.db'):
with open(os.path.join('tmp', filename), 'rb') as infp:
data = infp.read(BLOCK_SIZE)
print(time.perf_counter() - start)
Before each run, I wipe the file cache with echo 1 >
/proc/sys/vm/drop_caches).
------------------------------
With a bunch of free memory (~ 26 GB) and a read size of 65535 bytes:
$ ls tmp/ | wc -l
60000
$ free
total used free shared buff/cache available
Mem: 32537864 5188908 26183948 668028 1165008 26234928
Swap: 0 0 0
$ python read_simple.py 65535
BLOCK_SIZE 65535
12.427678888998344
0.9561608399963006
0.8500389300024835
0.8534953799971845
0.8507285489904461
0.8569888329948299
0.8777932550001424
0.8816033009934472
0.8621571760013467
0.8545428220095346
With a bunch of free memory (~ 26 GB) and a read size of 4096 bytes:
(prometheus_profile) ***@***.*** prometheus_profile]$ free
total used free shared buff/cache available
Mem: 32537864 5148264 26250408 645920 1139192 26298164
Swap: 0 0 0
(prometheus_profile) ***@***.*** prometheus_profile]$ python read_simple.py 4096
BLOCK_SIZE 4096
10.32166458798747
0.4410311039973749
0.39926853599899914
0.4384756700019352
0.4004494149994571
0.40352619100303855
0.3965474760043435
0.3989230669976678
0.39654788600455504
0.4002341330051422
Now with ~ 2 GB of free memory and a read size of 65535 bytes:
$ free
total used free shared buff/cache available
Mem: 32537864 29285200 2293668 663724 958996 2171332
Swap: 0 0 0
$ python read_simple.py 65535
BLOCK_SIZE 65535
12.443866290006554
11.633168715998181
12.085821863001911
11.749885090990574
12.8851961879991
12.385939428990241
11.35605888698774
11.659072276990628
11.294882508998853
11.368941648004693
And finally with ~ 2 GB of free memory and a read size of 4096 bytes:
$ free
total used free shared buff/cache available
Mem: 32537864 29260832 2405596 646916 871436 2247648
Swap: 0 0 0
$ python read_simple.py 4096
BLOCK_SIZE 4096
12.362105008010985
0.40657658501004335
0.3870056059968192
0.4089814879989717
0.381813013998908
0.3841210550017422
0.38265788700664416
0.38066420500399545
0.3836389770003734
0.3772799519938417
And sure enough, if I constraint the system even more to have less than a
500 MB of free memory, the performance is bad even with 4096:
(prometheus_profile) ***@***.*** prometheus_profile]$ free
total used free shared buff/cache available
Mem: 32537864 31400812 335304 621992 801748 155100
Swap: 0 0 0
(prometheus_profile) ***@***.*** prometheus_profile]$ python read_simple.py 4096
BLOCK_SIZE 4096
11.380508398986422
10.212263110995991
12.595423141989158
11.597996773009072
9.844564571001683
11.246935460992972
11.166533361989423
10.640836154998397
10.942926752992207
11.625870076008141
And in the same way, even with 25 GB of available memory, if I push the
read size to 650 350 kbytes:
$ free
total used free shared buff/cache available
Mem: 32537864 5259756 26470680 632860 807428 26287820
Swap: 0 0 0
$ python read_simple.py 655350
BLOCK_SIZE 655350
22.036876083991956
20.698863850993803
23.11240423300478
21.609464114997536
21.656152483003098
22.610070716997143
22.320307180998498
24.78865629399661
23.689863965002587
20.960628273998736
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#505?email_source=notifications&email_token=ABWJG5TXIIDU2RLP5P6GV2TRALZKJA5CNFSM4KNLPZI2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEKLO54Q#issuecomment-580316914>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABWJG5SY4KYW36UHG7YFZVTRALZKJANCNFSM4KNLPZIQ>
.
|
|
But why 4k and not 1k or 8k ? Depending on the number of files, the number of bytes used and the available memory, surely the optimum value might differ ? |
|
4k is the page size, so Iower than that won't help. The performance numbers for 4k are acceptable, so I don't see a need to consider 8k - unless you've more data. |
This size can have a real impact on performance when a lot of files need to be read. The performance relies heavily on the file system cache so reading as little pages as possible helps a lot on memory constrained systems. Signed-off-by: Xavier Fernandez <xavier.fernandez@polyconseil.fr>
I tried to generate less but bigger files with: The resulting files ended up using ~ 58kB. And I updated the (which means 4096 will require a second read but 65535 won't). With very little RAM, the performance is slightly better with 65535 but not dramatically so: And with lot of RAM, as expected, there is little impact: So 4096 seems like a good compromise 👍 I updated the PR to use |
xavfernandez
force-pushed
the
xfernandez/extract_constant
branch
from
695f095
to
e7874ee
Compare
xavfernandez
changed the title
|
Yip, that seems reasonable. Thanks! |
|
Thanks for merging :) |
|
I don't think there's anything pressing for release currently, I'll probably wait until the OpenMetrics stuff is a bit more finalised. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This size can have a real impact on performance when a lot of files need
to be read.
The performance relies heavily on the file system cache and having the
possibility to adapt _INITIAL_READ_SIZE can help on memory constrained
systems.