[haiku-commits] haiku: hrev53246 - in src: system/kernel/cache system/kernel preferences/filetypes

  • From: waddlesplash <waddlesplash@xxxxxxxxx>
  • To: haiku-commits@xxxxxxxxxxxxx
  • Date: Fri, 12 Jul 2019 21:22:51 -0400 (EDT)

hrev53246 adds 3 changesets to branch 'master'
old head: 9d06690edec597ce6828c02b1f657b5f111ba9b6
new head: 19d1cffe33373572da33f477596411ce4e0cb965
overview: 
https://git.haiku-os.org/haiku/log/?qt=range&q=19d1cffe3337+%5E9d06690edec5

----------------------------------------------------------------------------

6d336fda4aca: block_cache: Compute the timeout dynamically based on write 
speeds.
  
  We now record how long it takes to write a block (on average), and then
  utilize this information to reduce the timeout write thread's timeout
  (to 2 * block_count * average_block_time, so we don't completely
  congest the drive.) Remove the "TODO" about the I/O scheduler;
  this new logic will be just fine even under an I/O scheduler.
  
  Note that this change goes both ways: while faster writes mean more
  writes and quicker, slower writes will increase the timeout before
  we do another one also. This then also guards against queueing
  another write while one is already in progress, which was
  not handled before.
  
  Tested in KVM. Even on a SATA-backed spinning HDD, this reduces
  the timeout to around *200ms* on average (!!), so a 10x improvement.
  On a ramdisk, it reduces the timeout to *10-30ms* (!!!) on average,
  so a 100-200x improvement, so this change will benefit everyone
  but SSDs especially.
  
  Since BFS inode and journal writes always go through the block_cache,
  this very dramatically improves inode-related write performance.
  The "stop and start" stutters when emptying or moving items to Trash
  seem totally gone, among a lot of other things.
  
  Change-Id: I41f46a6432ce1f50f896a853abdfe22dde0ba327

fa146526b615: kernel/system_info: Avoid malloc() in _user_get_cpu_info.
  
  This is called 15-30 times per second (15 by Pulse, 30 by ActivityMonitor),
  so we want to keep it as low-profile as is possible.

19d1cffe3337: FileTypes: Prevent a potential snprintf overflow.

                              [ Augustin Cavalier <waddlesplash@xxxxxxxxx> ]

----------------------------------------------------------------------------

3 files changed, 69 insertions(+), 26 deletions(-)
src/preferences/filetypes/AttributeListView.cpp |  2 +-
src/system/kernel/cache/block_cache.cpp         | 58 ++++++++++++++++++---
src/system/kernel/system_info.cpp               | 35 +++++++------

############################################################################

Commit:      6d336fda4aca32649de3e1d91403da4452f4bef8
URL:         https://git.haiku-os.org/haiku/commit/?id=6d336fda4aca
Author:      Augustin Cavalier <waddlesplash@xxxxxxxxx>
Date:        Thu Jul 11 00:43:32 2019 UTC

block_cache: Compute the timeout dynamically based on write speeds.

We now record how long it takes to write a block (on average), and then
utilize this information to reduce the timeout write thread's timeout
(to 2 * block_count * average_block_time, so we don't completely
congest the drive.) Remove the "TODO" about the I/O scheduler;
this new logic will be just fine even under an I/O scheduler.

Note that this change goes both ways: while faster writes mean more
writes and quicker, slower writes will increase the timeout before
we do another one also. This then also guards against queueing
another write while one is already in progress, which was
not handled before.

Tested in KVM. Even on a SATA-backed spinning HDD, this reduces
the timeout to around *200ms* on average (!!), so a 10x improvement.
On a ramdisk, it reduces the timeout to *10-30ms* (!!!) on average,
so a 100-200x improvement, so this change will benefit everyone
but SSDs especially.

Since BFS inode and journal writes always go through the block_cache,
this very dramatically improves inode-related write performance.
The "stop and start" stutters when emptying or moving items to Trash
seem totally gone, among a lot of other things.

Change-Id: I41f46a6432ce1f50f896a853abdfe22dde0ba327

----------------------------------------------------------------------------

diff --git a/src/system/kernel/cache/block_cache.cpp 
b/src/system/kernel/cache/block_cache.cpp
index 37147582ca..6b4ac88f1e 100644
--- a/src/system/kernel/cache/block_cache.cpp
+++ b/src/system/kernel/cache/block_cache.cpp
@@ -226,6 +226,9 @@ struct block_cache : DoublyLinkedListLinkImpl<block_cache> {
        uint32                  busy_writing_count;
        bool                    busy_writing_waiters;
 
+       bigtime_t               last_block_write;
+       bigtime_t               last_block_write_duration;
+
        uint32                  num_dirty_blocks;
        bool                    read_only;
 
@@ -1203,6 +1206,8 @@ BlockWriter::Write(cache_transaction* transaction, bool 
canUnlock)
        qsort(fBlocks, fCount, sizeof(void*), &_CompareBlocks);
        fDeletedTransaction = false;
 
+       bigtime_t start = system_time();
+
        for (uint32 i = 0; i < fCount; i++) {
                status_t status = _WriteBlock(fBlocks[i]);
                if (status != B_OK) {
@@ -1216,9 +1221,17 @@ BlockWriter::Write(cache_transaction* transaction, bool 
canUnlock)
                }
        }
 
+       bigtime_t finish = system_time();
+
        if (canUnlock)
                mutex_lock(&fCache->lock);
 
+       if (fStatus == B_OK && fCount >= 8) {
+               fCache->last_block_write = finish;
+               fCache->last_block_write_duration = (fCache->last_block_write - 
start)
+                       / fCount;
+       }
+
        for (uint32 i = 0; i < fCount; i++)
                _BlockDone(fBlocks[i], transaction);
 
@@ -1392,6 +1405,8 @@ block_cache::block_cache(int _fd, off_t numBlocks, size_t 
blockSize,
        busy_reading_waiters(false),
        busy_writing_count(0),
        busy_writing_waiters(0),
+       last_block_write(0),
+       last_block_write_duration(0),
        num_dirty_blocks(0),
        read_only(readOnly)
 {
@@ -2538,8 +2553,8 @@ get_next_locked_block_cache(block_cache* last)
 static status_t
 block_notifier_and_writer(void* /*data*/)
 {
-       const bigtime_t kTimeout = 2000000LL;
-       bigtime_t timeout = kTimeout;
+       const bigtime_t kDefaultTimeout = 2000000LL;
+       bigtime_t timeout = kDefaultTimeout;
 
        while (true) {
                bigtime_t start = system_time();
@@ -2552,15 +2567,32 @@ block_notifier_and_writer(void* /*data*/)
                        continue;
                }
 
-               // write 64 blocks of each block_cache every two seconds
-               // TODO: change this once we have an I/O scheduler
-               timeout = kTimeout;
+               // Write 64 blocks of each block_cache roughly every 2 seconds,
+               // potentially more or less depending on congestion and drive 
speeds
+               // (usually much less.) We do not want to queue everything at 
once
+               // because a future transaction might then get held up waiting 
for
+               // a specific block to be written.
+               timeout = kDefaultTimeout;
                size_t usedMemory;
                object_cache_get_usage(sBlockCache, &usedMemory);
 
                block_cache* cache = NULL;
                while ((cache = get_next_locked_block_cache(cache)) != NULL) {
+                       // Give some breathing room: wait 2x the length of the 
potential
+                       // maximum block count-sized write between writes, and 
also skip
+                       // if there are more than 16 blocks currently being 
written.
+                       const bigtime_t next = cache->last_block_write
+                                       + cache->last_block_write_duration * 2 
* 64;
+                       if (cache->busy_writing_count > 16 || system_time() < 
next) {
+                               if (cache->last_block_write_duration > 0) {
+                                       timeout = min_c(timeout,
+                                               
cache->last_block_write_duration * 2 * 64);
+                               }
+                               continue;
+                       }
+
                        BlockWriter writer(cache, 64);
+                       bool hasMoreBlocks = false;
 
                        size_t cacheUsedMemory;
                        object_cache_get_usage(cache->buffer_cache, 
&cacheUsedMemory);
@@ -2573,10 +2605,11 @@ block_notifier_and_writer(void* /*data*/)
 
                                while (iterator.HasNext()) {
                                        cached_block* block = iterator.Next();
-                                       if (block->CanBeWritten() && 
!writer.Add(block))
+                                       if (block->CanBeWritten() && 
!writer.Add(block)) {
+                                               hasMoreBlocks = true;
                                                break;
+                                       }
                                }
-
                        } else {
                                TransactionTable::Iterator 
iterator(cache->transaction_hash);
 
@@ -2594,13 +2627,22 @@ block_notifier_and_writer(void* /*data*/)
 
                                        bool hasLeftOvers;
                                                // we ignore this one
-                                       if (!writer.Add(transaction, 
hasLeftOvers))
+                                       if (!writer.Add(transaction, 
hasLeftOvers)) {
+                                               hasMoreBlocks = true;
                                                break;
+                                       }
                                }
                        }
 
                        writer.Write();
 
+                       if (hasMoreBlocks && cache->last_block_write_duration > 
0) {
+                               // There are probably still more blocks that we 
could write, so
+                               // see if we can decrease the timeout.
+                               timeout = min_c(timeout,
+                                       cache->last_block_write_duration * 2 * 
64);
+                       }
+
                        if ((block_cache_used_memory() / B_PAGE_SIZE)
                                        > vm_page_num_pages() / 2) {
                                // Try to reduce memory usage to half of the 
available

############################################################################

Commit:      fa146526b6159f735c349b63d1d54c5bc61c522e
URL:         https://git.haiku-os.org/haiku/commit/?id=fa146526b615
Author:      Augustin Cavalier <waddlesplash@xxxxxxxxx>
Date:        Sat Jul 13 01:22:09 2019 UTC

kernel/system_info: Avoid malloc() in _user_get_cpu_info.

This is called 15-30 times per second (15 by Pulse, 30 by ActivityMonitor),
so we want to keep it as low-profile as is possible.

----------------------------------------------------------------------------

diff --git a/src/system/kernel/system_info.cpp 
b/src/system/kernel/system_info.cpp
index 82431a94ea..ea181f630e 100644
--- a/src/system/kernel/system_info.cpp
+++ b/src/system/kernel/system_info.cpp
@@ -500,12 +500,28 @@ get_cpu_info(uint32 firstCPU, uint32 cpuCount, cpu_info* 
info)
 
        uint32 count = std::min(cpuCount, smp_get_num_cpus() - firstCPU);
 
-       memset(info, 0, sizeof(cpu_info) * count);
+       // This function is called very often from userland by applications
+       // that display CPU usage information, so we want to keep this as
+       // optimized and touch as little as possible. Hence, no use of
+       // a temporary buffer.
+
+       if (IS_USER_ADDRESS(info)) {
+               if (user_memset(info, 0, sizeof(cpu_info) * count) != B_OK)
+                       return B_BAD_ADDRESS;
+               set_ac();
+       } else {
+               memset(info, 0, sizeof(cpu_info) * count);
+       }
+
        for (uint32 i = 0; i < count; i++) {
                info[i].active_time = cpu_get_active_time(firstCPU + i);
                info[i].enabled = !gCPU[firstCPU + i].disabled;
        }
 
+       if (IS_USER_ADDRESS(info)) {
+               clear_ac();
+       }
+
        return B_OK;
 }
 
@@ -561,23 +577,8 @@ _user_get_cpu_info(uint32 firstCPU, uint32 cpuCount, 
cpu_info* userInfo)
 {
        if (userInfo == NULL || !IS_USER_ADDRESS(userInfo))
                return B_BAD_ADDRESS;
-       if (firstCPU >= (uint32)smp_get_num_cpus())
-               return B_BAD_VALUE;
-       if (cpuCount == 0)
-               return B_OK;
-
-       uint32 count = std::min(cpuCount, smp_get_num_cpus() - firstCPU);
-
-       cpu_info* cpuInfos = new(std::nothrow) cpu_info[count];
-       if (cpuInfos == NULL)
-               return B_NO_MEMORY;
-       ArrayDeleter<cpu_info> _(cpuInfos);
-
-       status_t error = get_cpu_info(firstCPU, count, cpuInfos);
-       if (error != B_OK)
-               return error;
 
-       return user_memcpy(userInfo, cpuInfos, sizeof(cpu_info) * count);
+       return get_cpu_info(firstCPU, cpuCount, userInfo);
 }
 
 

############################################################################

Revision:    hrev53246
Commit:      19d1cffe33373572da33f477596411ce4e0cb965
URL:         https://git.haiku-os.org/haiku/commit/?id=19d1cffe3337
Author:      Augustin Cavalier <waddlesplash@xxxxxxxxx>
Date:        Sat Jul 13 01:22:28 2019 UTC

FileTypes: Prevent a potential snprintf overflow.

----------------------------------------------------------------------------

diff --git a/src/preferences/filetypes/AttributeListView.cpp 
b/src/preferences/filetypes/AttributeListView.cpp
index fce417a9db..fec36dc8cf 100644
--- a/src/preferences/filetypes/AttributeListView.cpp
+++ b/src/preferences/filetypes/AttributeListView.cpp
@@ -104,7 +104,7 @@ name_for_type(BString& string, type_code type, const char* 
displayAs)
                        buffer[i] = '.';
        }
 
-       snprintf(buffer + 6, sizeof(buffer), " (0x%" B_PRIx32 ")", type);
+       snprintf(buffer + 6, sizeof(buffer) - 6, " (0x%" B_PRIx32 ")", type);
        string = buffer;
 }
 


Other related posts:

  • » [haiku-commits] haiku: hrev53246 - in src: system/kernel/cache system/kernel preferences/filetypes - waddlesplash