This is an automated email from the git hooks/post-receive script. x2go pushed a commit to branch 3.6.x in repository nx-libs. commit 99a2d6e292a56ad3756348e5b8d997a2b75c7539 Author: Ulrich Sibiller <uli42@gmx.de> Date: Wed Nov 18 22:41:32 2020 +0100 Clipboard.c: implement target caching Some applications (e.g. Chrome, Firefox) send multiple TARGETS requests when they want to paste a selection (reason currently unknown). If such an application runs in an NX session and the current clipboard owner is on the real X server this triggers many (superflous) roundtrips. By caching the first answer to those requests and answering subsequent requests from the cache we can eliminate all this communication. The implemention resulted from attempting to fix ArcticaProject/nx-libs/issues/#969 (solution 1a) but it turned out that the speedup this brings does not help in reducing the number of requests by firefox. It must be a bug in the firefox code. Nevertheless I kept the code as it should speed up the whole pasting process considerably while reducing communication over the wire. This also eliminates a memory leak in the target forwarding code which was unnoticed before. --- nx-X11/programs/Xserver/hw/nxagent/Clipboard.c | 118 +++++++++++++++++++++++++ 1 file changed, 118 insertions(+) diff --git a/nx-X11/programs/Xserver/hw/nxagent/Clipboard.c b/nx-X11/programs/Xserver/hw/nxagent/Clipboard.c index f8d1a5d18..18bd44099 100644 --- a/nx-X11/programs/Xserver/hw/nxagent/Clipboard.c +++ b/nx-X11/programs/Xserver/hw/nxagent/Clipboard.c @@ -106,6 +106,19 @@ typedef struct _SelectionOwner static SelectionOwner *lastSelectionOwner = NULL; +/* + * cache for targets the current selection owner + * on the real X server has to offer. We are storing the targets + * after they have been converted from XlibAtom to Atom. +*/ +typedef struct _Targets +{ + Atom *targets; + int numTargets; +} Targets; + +static Targets *targetCache = NULL; + /* FIXME: can this also be stored per selection? */ static XlibAtom serverLastRequestedSelection = -1; @@ -378,6 +391,12 @@ static void printLastServerStat(int index) fprintf(stderr, " lastServer[].time (Time) [%u]\n", ls.time); } +static void printTargetCacheStat(int index) +{ + fprintf(stderr, " targetCache[].targets (Atom *) [%p]\n", (void *)targetCache[index].targets); + fprintf(stderr, " targetCache[].numTargets (int) [%d]\n", targetCache[index].numTargets); +} + void nxagentDumpClipboardStat(void) { fprintf(stderr, "/----- Clipboard internal status -----\n"); @@ -452,6 +471,7 @@ void nxagentDumpClipboardStat(void) printSelectionStat(index); printLastClientStat(index); printLastServerStat(index); + printTargetCacheStat(index); } fprintf(stderr, "\\------------------------------------------------------------------------------\n"); @@ -747,6 +767,41 @@ int nxagentFindCurrentSelectionIndex(Atom sel) return NumCurrentSelections; } +void cacheTargets(int index, Atom* targets, int numTargets) +{ + #ifdef DEBUG + fprintf(stderr, "%s: caching [%d] targets\n", __func__, numTargets); + #endif + + SAFE_free(targetCache[index].targets); + targetCache[index].targets = targets; + targetCache[index].numTargets = numTargets; +} + +/* this is called on init, reconnect and SelectionClear */ +void invalidateTargetCache(int index) +{ + #ifdef DEBUG + fprintf(stderr, "%s: invalidating target cache [%d]\n", __func__, index); + #endif + + SAFE_free(targetCache[index].targets); + targetCache[index].numTargets = 0; +} + +void invalidateTargetCaches(void) +{ + #ifdef DEBUG + fprintf(stderr, "%s: invalidating all target caches\n", __func__); + #endif + + for (int index = 0; index < nxagentMaxSelections; index++) + { + SAFE_free(targetCache[index].targets); + targetCache[index].numTargets = 0; + } +} + /* * This is called from Events.c dispatch loop on reception of a * SelectionClear event. We receive this event if someone on the real @@ -802,6 +857,8 @@ void nxagentHandleSelectionClearFromXServer(XEvent *X) clearSelectionOwnerData(index); setClientSelectionStage(SelectionStageNone, index); + + invalidateTargetCache(index); } } @@ -1543,6 +1600,8 @@ Bool nxagentCollectPropertyEventFromXServer(int resource) 32, PropModeReplace, ulReturnItems, (unsigned char*)targets, 1); + cacheTargets(index, targets, numTargets); + endTransfer(SELECTION_SUCCESS, index); } } @@ -1951,6 +2010,12 @@ void nxagentSetSelectionCallback(CallbackListPtr *callbacks, void *data, } #endif + /* + * We do not know index here so for now let's invalidate the + * complete cache on every owner change regardless of the selection. + */ + invalidateTargetCaches(); + if (nxagentExternalClipboardEventTrap) { #ifdef DEBUG @@ -2063,6 +2128,13 @@ static void setSelectionOwnerOnXServer(Selection *pSelection) storeSelectionOwnerData(index, pSelection); setClientSelectionStage(SelectionStageNone, index); + + /* + * this will be repeated on reception of the SelectionOwner callback + * but we cannot be sure if there are any intermediate requests in the queue + * already so better do it here, too + */ + invalidateTargetCache(index); } /* FIXME: commented because index is invalid here! */ @@ -2253,6 +2325,42 @@ int nxagentConvertSelection(ClientPtr client, WindowPtr pWin, Atom selection, } else { + /* + * Shortcut: Some applications tend to post multiple + * SelectionRequests. Further it can happen that multiple + * clients are interested in clipboard content. If we already + * know the answer and no intermediate SelectionOwner event + * occured we can answer with the cached list of targets. + */ + + if (targetCache[index].targets) + { + Atom *targets = targetCache[index].targets; + int numTargets = targetCache[index].numTargets; + + #ifdef DEBUG + fprintf(stderr, "%s: Sending %d cached targets:\n", __func__, numTargets); + for (int i = 0; i < numTargets; i++) + { + fprintf(stderr, "%s: %d %s\n", __func__, targets[i], NameForIntAtom(targets[i])); + } + #endif + + ChangeWindowProperty(pWin, + property, + MakeAtom("ATOM", 4, 1), + sizeof(Atom)*8, + PropModeReplace, + numTargets, + targets, + 1); + + sendSelectionNotifyEventToClient(client, time, requestor, selection, + target, property); + + return 1; + } + /* * do nothing - TARGETS will be handled like any other target * and passed on to the owner on the remote side. @@ -2760,6 +2868,13 @@ Bool nxagentInitClipboard(WindowPtr pWin) { FatalError("nxagentInitClipboard: Failed to allocate memory for the remote selection Atoms array.\n"); } + + SAFE_free(targetCache); + targetCache = (Targets *) calloc(nxagentMaxSelections, sizeof(Targets)); + if (targetCache == NULL) + { + FatalError("nxagentInitClipboard: Failed to allocate memory for target cache.\n"); + } } /* @@ -2884,6 +2999,8 @@ Bool nxagentInitClipboard(WindowPtr pWin) * clients might still be waiting for answers. Should reply * with failure then */ + + invalidateTargetCache(index); } } } @@ -2896,6 +3013,7 @@ Bool nxagentInitClipboard(WindowPtr pWin) /* FIXME: required? move to setSelectionStage? */ lastClients[index].reqTime = GetTimeInMillis(); lastServers[index].requestor = None; + invalidateTargetCache(index); } clientCutProperty = MakeAtom(szAgentNX_CUT_BUFFER_CLIENT, -- Alioth's /home/x2go-admin/maintenancescripts/git/hooks/post-receive-email on /srv/git/code.x2go.org/nx-libs.git