Optimize c-readerchannel (#21)

chpock · Jun 2, 2024 · 199a040 · 199a040
1 parent 077bab3
commit 199a040
Show file tree

Hide file tree

Showing 8 changed files with 203 additions and 79 deletions.
diff --git a/ChangeLog b/ChangeLog
@@ -1,5 +1,6 @@
 2024-06-02 Konstantin Kushnir <[email protected]>
 	* Don't allow deleting pages/fsindex that are in use
+	* Optimize c-readerchannel
 
 2024-06-01 Konstantin Kushnir <[email protected]>
 	* Add in-memory vfs feature

diff --git a/generic/readerchannel.c b/generic/readerchannel.c
@@ -10,8 +10,6 @@
 #include "cookfs.h"
 #include <errno.h>
 
-static int cookfsChannelId = 0;
-
 static Tcl_ChannelType cookfsReaderChannel = {
     "cookfsreader",
     TCL_CHANNEL_VERSION_5,
@@ -36,7 +34,7 @@ static Tcl_ChannelType cookfsReaderChannel = {
     NULL,
     NULL,
     Cookfs_Readerchannel_WideSeek,
-    NULL,
+    Cookfs_Readerchannel_ThreadAction,
     NULL,
 };
 
@@ -115,16 +113,18 @@ Tcl_Channel Cookfs_CreateReaderchannel(Cookfs_Pages *pages, Cookfs_Fsindex *fsin
 
     if (channelNamePtr != NULL)
     {
-	*channelNamePtr = instData->channelName;
+	*channelNamePtr = (char *)Tcl_GetChannelName(instData->channel);
     }
 
     return instData->channel;
 }
 
 int Cookfs_CreateReaderchannelCreate(Cookfs_ReaderChannelInstData *instData, Tcl_Interp *interp) {
-    /* TODO: mutex */
-    sprintf(instData->channelName, "cookfsreader%d", ++cookfsChannelId);
-    instData->channel = Tcl_CreateChannel(&cookfsReaderChannel, instData->channelName, (ClientData) instData, TCL_READABLE);
+    char channelName[128];
+    sprintf(channelName, "cookfsreader%p", (void *)instData);
+
+    instData->channel = Tcl_CreateChannel(&cookfsReaderChannel, channelName,
+        (ClientData) instData, TCL_READABLE);
 
     if (instData->channel == NULL) {
 	CookfsLog(printf("Cookfs_CreateReaderchannelCreate: Unable to create channel"))
@@ -143,7 +143,7 @@ Cookfs_ReaderChannelInstData *Cookfs_CreateReaderchannelAlloc(Cookfs_Pages *page
 
     result = (Cookfs_ReaderChannelInstData *) ckalloc(sizeof(Cookfs_ReaderChannelInstData) + bufSize * sizeof(int));
     result->channel = NULL;
-    result->watchTimer = NULL;
+    result->event = NULL;
 
     result->pages = pages;
     result->fsindex = fsindex;
@@ -155,21 +155,24 @@ Cookfs_ReaderChannelInstData *Cookfs_CreateReaderchannelAlloc(Cookfs_Pages *page
     result->fileSize = 0;
     result->bufSize = bufSize;
 
+    result->cachedPageObj = NULL;
+
     result->firstTimeRead = 1;
 
     return result;
 }
 
 void Cookfs_CreateReaderchannelFree(Cookfs_ReaderChannelInstData *instData) {
-    CookfsLog(printf("Cookfs_CreateReaderchannelFree: freeing channel=%s\n", instData->channelName))
-    strcpy(instData->channelName, "INVALID");
-    if (instData->watchTimer != NULL) {
-    	CookfsLog(printf("Cookfs_CreateReaderchannelFree: deleting target"))
-	Tcl_DeleteTimerHandler(instData->watchTimer);
+    CookfsLog(printf("Cookfs_CreateReaderchannelFree: freeing channel=%s",
+        Tcl_GetChannelName(instData->channel)));
+    if (instData->event != NULL) {
+        instData->event->instData = NULL;
+        instData->event = NULL;
+    }
+    if (instData->cachedPageObj != NULL) {
+        Tcl_DecrRefCount(instData->cachedPageObj);
     }
-    CookfsLog(printf("Cookfs_CreateReaderchannelFree: before free"))
     ckfree((void *) instData);
-    CookfsLog(printf("Cookfs_CreateReaderchannelFree: after free"))
 }
 
 /* command for creating new objects that deal with pages */

diff --git a/generic/readerchannel.h b/generic/readerchannel.h
@@ -7,12 +7,16 @@
 #define COOKFS_READERCHANNEL_H 1
 
 #ifdef COOKFS_USECREADERCHAN
+
+typedef struct Cookfs_ReaderChannelEvent {
+  Tcl_Event header;
+  struct Cookfs_ReaderChannelInstData *instData;
+} Cookfs_ReaderChannelEvent;
+
 typedef struct Cookfs_ReaderChannelInstData {
-    // "cookfsreader%d"+\0 where "%d" is between 1 and 11 bytes.
-    // The buffer should be 12+11+1=24 bytes.
-    char channelName[24];
     Tcl_Channel channel;
-    Tcl_TimerToken watchTimer;
+    Cookfs_ReaderChannelEvent *event;
+    int interest;
 
     Cookfs_Pages *pages;
     Cookfs_Fsindex *fsindex;
@@ -23,6 +27,9 @@ typedef struct Cookfs_ReaderChannelInstData {
     int currentBlockOffset;
     int firstTimeRead;
 
+    Tcl_Obj *cachedPageObj;
+    int cachedPageNum;
+
     int bufSize;
     int buf[1];
 } Cookfs_ReaderChannelInstData;

diff --git a/generic/readerchannelIO.c b/generic/readerchannelIO.c
@@ -12,7 +12,8 @@
 int Cookfs_Readerchannel_Close(ClientData instanceData, Tcl_Interp *interp) {
     UNUSED(interp);
     Cookfs_ReaderChannelInstData *instData = (Cookfs_ReaderChannelInstData *) instanceData;
-    CookfsLog(printf("Cookfs_Readerchannel_Close: channel=%s\n", instData->channelName))
+    CookfsLog(printf("Cookfs_Readerchannel_Close: channel=%s",
+        Tcl_GetChannelName(instData->channel)));
     Cookfs_CreateReaderchannelFree(instData);
     return 0;
 }
@@ -27,15 +28,14 @@ int Cookfs_Readerchannel_Close2(ClientData instanceData, Tcl_Interp *interp, int
 
 int Cookfs_Readerchannel_Input(ClientData instanceData, char *buf, int bufSize, int *errorCodePtr) {
     Cookfs_ReaderChannelInstData *instData = (Cookfs_ReaderChannelInstData *) instanceData;
-    Tcl_Obj *pageObj;
     int bytesRead = 0;
     int bytesLeft;
     int blockLeft;
     int blockRead;
     char *pageBuf;
     Tcl_Size pageBufSize;
 
-    CookfsLog(printf("Cookfs_Readerchannel_Input: read %d, current offset: %"
+    CookfsLog(printf("Cookfs_Readerchannel_Input: ===> read %d, current offset: %"
         TCL_LL_MODIFIER "d", bufSize, instData->currentOffset))
 
     if (instData->currentBlock >= instData->bufSize) {
@@ -65,7 +65,17 @@ int Cookfs_Readerchannel_Input(ClientData instanceData, char *buf, int bufSize,
 	}
 
 	int pageIndex = instData->buf[instData->currentBlock + 0];
-	CookfsLog(printf("Cookfs_Readerchannel_Input: reading page %d", pageIndex))
+
+	if (instData->cachedPageObj != NULL) {
+	    if (instData->cachedPageNum == pageIndex) {
+		CookfsLog(printf("Cookfs_Readerchannel_Input: use"
+		    " the previously retrieved page index#%d", pageIndex));
+		goto gotPage;
+	    }
+	    Tcl_DecrRefCount(instData->cachedPageObj);
+	}
+
+	CookfsLog(printf("Cookfs_Readerchannel_Input: reading page index#%d", pageIndex));
 	int pageUsage = Cookfs_FsindexGetBlockUsage(instData->fsindex, pageIndex);
 	/* If page contains only one file, set its weight to 0. Otherwise, set its weight to 1. */
 	int pageWeight = (pageUsage <= 1) ? 0 : 1;
@@ -85,31 +95,43 @@ int Cookfs_Readerchannel_Input(ClientData instanceData, char *buf, int bufSize,
 	    }
 	    instData->firstTimeRead = 0;
 	}
-	pageObj = Cookfs_PageGet(instData->pages, pageIndex, pageWeight);
-	CookfsLog(printf("Cookfs_Readerchannel_Input: result %s", (pageObj ? "SET" : "NULL")))
-
-	if (pageObj == NULL) {
+	instData->cachedPageObj = Cookfs_PageGet(instData->pages, pageIndex, pageWeight);
+	// No need to increase refcount on retrieved page, it already has refcount=1
+	// after Cookfs_PageGet().
+	CookfsLog(printf("Cookfs_Readerchannel_Input: got the page: %p",
+	    (void *)instData->cachedPageObj))
+	if (instData->cachedPageObj == NULL) {
 	    goto error;
 	}
+	instData->cachedPageNum = pageIndex;
+
+gotPage:
 
-	pageBuf = (char *) Tcl_GetByteArrayFromObj(pageObj, &pageBufSize);
+	pageBuf = (char *) Tcl_GetByteArrayFromObj(instData->cachedPageObj, &pageBufSize);
 
 	CookfsLog(printf("Cookfs_Readerchannel_Input: copying %d+%d", instData->buf[instData->currentBlock + 1], instData->currentBlockOffset))
 	// validate enough data is available in the buffer
 	if (pageBufSize < (instData->buf[instData->currentBlock + 1] + instData->currentBlockOffset + blockRead)) {
 	    goto error;
 	}
 	memcpy(buf + bytesRead, pageBuf + instData->buf[instData->currentBlock + 1] + instData->currentBlockOffset, blockRead);
-	Tcl_DecrRefCount(pageObj);
-
 	instData->currentBlockOffset += blockRead;
 	bytesRead += blockRead;
 	instData->currentOffset += blockRead;
 	CookfsLog(printf("Cookfs_Readerchannel_Input: currentOffset: %"
 	    TCL_LL_MODIFIER "d", instData->currentOffset));
+	// If we have reached the end of the current page, we probably don't need
+	// it anymore and can release it.
+	if (instData->currentBlockOffset == instData->buf[instData->currentBlock + 2]) {
+	    CookfsLog(printf("Cookfs_Readerchannel_Input: release the page"));
+	    Tcl_DecrRefCount(instData->cachedPageObj);
+	    instData->cachedPageObj = NULL;
+	} else {
+	    CookfsLog(printf("Cookfs_Readerchannel_Input: keep the page"));
+	}
     }
 
-    CookfsLog(printf("Cookfs_Readerchannel_Input: bytesRead=%d", bytesRead))
+    CookfsLog(printf("Cookfs_Readerchannel_Input: <=== bytesRead=%d", bytesRead))
     return bytesRead;
 
 error:
@@ -179,27 +201,88 @@ int Cookfs_Readerchannel_Seek(ClientData instanceData, long offset, int seekMode
     return Cookfs_Readerchannel_WideSeek(instanceData, offset, seekMode, errorCodePtr);
 }
 
-/* watch implementation taken from rechan.c */
-static void CookfsReaderchannelTimerProc(ClientData instanceData)
-{
-    Cookfs_ReaderChannelInstData *chan = (Cookfs_ReaderChannelInstData *) instanceData;
-    if (chan->watchTimer != NULL) {
-	Tcl_DeleteTimerHandler(chan->watchTimer);
-	chan->watchTimer = NULL;
+void Cookfs_Readerchannel_ThreadAction(ClientData instanceData, int action) {
+    Cookfs_ReaderChannelInstData *instData =
+        (Cookfs_ReaderChannelInstData *) instanceData;
+
+    if (instData->channel == NULL) {
+        CookfsLog(printf("Cookfs_Readerchannel_ThreadAction: channel [NULL] at [%p]"
+            " action [%d]", (void *)instData, action));
+    } else {
+        CookfsLog(printf("Cookfs_Readerchannel_ThreadAction: channel [%s] at [%p]"
+            " action [%d]", Tcl_GetChannelName(instData->channel),
+            (void *)instData, action));
+    }
+
+    if (action == TCL_CHANNEL_THREAD_REMOVE) {
+        if (instData->event != NULL) {
+            instData->event->instData = NULL;
+            instData->event = NULL;
+        }
+        instData->interest = 0;
     }
-    Tcl_NotifyChannel(chan->channel, TCL_READABLE);
+}
+
+static int Cookfs_Readerchannel_Ready(Tcl_Event* evPtr, int flags) {
+    Cookfs_ReaderChannelInstData *instData =
+        ((Cookfs_ReaderChannelEvent *)evPtr)->instData;
+
+    if (instData == NULL) {
+        CookfsLog(printf("Cookfs_Readerchannel_Ready: NULL data"));
+        return 1;
+    }
+
+    CookfsLog(printf("Cookfs_Readerchannel_Ready: channel [%s] at [%p]"
+        " flags [%d]", Tcl_GetChannelName(instData->channel),
+        (void *)instData, flags));
+
+    if (!(flags & TCL_FILE_EVENTS)) {
+        CookfsLog(printf("Cookfs_Readerchannel_Ready: not TCL_FILE_EVENTS"));
+        return 0;
+    }
+
+    instData->event = NULL;
+
+    if (instData->interest) {
+        CookfsLog(printf("Cookfs_Readerchannel_Ready: call Tcl_NotifyChannel"
+            " with mask [%d]", instData->interest));
+        Tcl_NotifyChannel(instData->channel, instData->interest);
+    } else {
+        CookfsLog(printf("Cookfs_Readerchannel_Ready: interest is zero"));
+    }
+
+    return 1;
 }
 
 void Cookfs_Readerchannel_Watch(ClientData instanceData, int mask) {
-    Cookfs_ReaderChannelInstData *chan = (Cookfs_ReaderChannelInstData *) instanceData;
-    CookfsLog(printf("Cookfs_Readerchannel_Watch: channel=%s mask=%08x\n", chan->channelName, mask))
-    if (mask & TCL_READABLE) {
-        if (chan->watchTimer == NULL) {
-    	    chan->watchTimer = Tcl_CreateTimerHandler(5, CookfsReaderchannelTimerProc, instanceData);
+
+    Cookfs_ReaderChannelInstData *instData =
+        (Cookfs_ReaderChannelInstData *)instanceData;
+
+    CookfsLog(printf("Cookfs_Readerchannel_Watch: channel=%s mask=%08x",
+        Tcl_GetChannelName(instData->channel), mask))
+
+    instData->interest = mask;
+
+    if ((mask & TCL_READABLE) == 0) {
+        if (instData->event != NULL) {
+            instData->event->instData = NULL;
+            instData->event = NULL;
         }
-    } else if (chan->watchTimer != NULL) {
-	Tcl_DeleteTimerHandler(chan->watchTimer);
-	chan->watchTimer = NULL;
+        return;
     }
+
+    if (instData->event == NULL) {
+        instData->event = (Cookfs_ReaderChannelEvent*)ckalloc(
+            sizeof(Cookfs_ReaderChannelEvent));
+        if (instData->event == NULL) {
+            return;
+        }
+        instData->event->header.proc = Cookfs_Readerchannel_Ready;
+        instData->event->instData = instData;
+        Tcl_QueueEvent(&instData->event->header, TCL_QUEUE_TAIL);
+    }
+    CookfsLog(printf("Cookfs_Readerchannel_Watch: ok"));
+
 }
 
diff --git a/generic/readerchannelIO.h b/generic/readerchannelIO.h
@@ -1,23 +1,25 @@
-/* (c) 2010 Wojciech Kocjan, Pawel Salawa */
-
-#ifndef COOKFS_READERCHANNELIO_H
-#define COOKFS_READERCHANNELIO_H 1
-
-#ifdef COOKFS_USECREADERCHAN
-int Cookfs_Readerchannel_Close(ClientData instanceData, Tcl_Interp *interp);
-int Cookfs_Readerchannel_Close2(ClientData instanceData, Tcl_Interp *interp, int flags);
-
-int Cookfs_Readerchannel_Gethandle(ClientData instanceData, int direction, ClientData *handlePtr);
-
-int Cookfs_Readerchannel_Input(ClientData instanceData, char *buf, int bufSize, int *errorCodePtr);
-int Cookfs_Readerchannel_Output(ClientData instanceData, const char *buf, int toWrite, int *errorCodePtr);
-
-int Cookfs_Readerchannel_Seek(ClientData instanceData, long offset, int seekMode, int *errorCodePtr);
-Tcl_WideInt Cookfs_Readerchannel_WideSeek(ClientData instanceData, Tcl_WideInt offset, int seekMode, int *errorCodePtr);
-
-void Cookfs_Readerchannel_Watch(ClientData instanceData, int mask);
-
-#endif /* COOKFS_USECREADERCHAN */
-
-#endif /* COOKFS_READERCHANNELIO_H */
-
+/* (c) 2010 Wojciech Kocjan, Pawel Salawa */
+
+#ifndef COOKFS_READERCHANNELIO_H
+#define COOKFS_READERCHANNELIO_H 1
+
+#ifdef COOKFS_USECREADERCHAN
+int Cookfs_Readerchannel_Close(ClientData instanceData, Tcl_Interp *interp);
+int Cookfs_Readerchannel_Close2(ClientData instanceData, Tcl_Interp *interp, int flags);
+
+int Cookfs_Readerchannel_Gethandle(ClientData instanceData, int direction, ClientData *handlePtr);
+
+int Cookfs_Readerchannel_Input(ClientData instanceData, char *buf, int bufSize, int *errorCodePtr);
+int Cookfs_Readerchannel_Output(ClientData instanceData, const char *buf, int toWrite, int *errorCodePtr);
+
+int Cookfs_Readerchannel_Seek(ClientData instanceData, long offset, int seekMode, int *errorCodePtr);
+Tcl_WideInt Cookfs_Readerchannel_WideSeek(ClientData instanceData, Tcl_WideInt offset, int seekMode, int *errorCodePtr);
+
+void Cookfs_Readerchannel_Watch(ClientData instanceData, int mask);
+
+Tcl_DriverThreadActionProc Cookfs_Readerchannel_ThreadAction;
+
+#endif /* COOKFS_USECREADERCHAN */
+
+#endif /* COOKFS_READERCHANNELIO_H */
+
diff --git a/generic/writerchannel.h b/generic/writerchannel.h
@@ -5,15 +5,15 @@
 #ifndef COOKFS_WRITERCHANNEL_H
 #define COOKFS_WRITERCHANNEL_H 1
 
-typedef struct ChannelEvent {
+typedef struct Cookfs_WriterChannelEvent {
   Tcl_Event header;
   struct Cookfs_WriterChannelInstData *instData;
-} ChannelEvent;
+} Cookfs_WriterChannelEvent;
 
 typedef struct Cookfs_WriterChannelInstData {
     Tcl_Channel channel;
     Tcl_Interp *interp;
-    ChannelEvent* event;
+    Cookfs_WriterChannelEvent* event;
     int interest;
     Tcl_Obj *closeResult;