Skip to content
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

Memory Leak while interacting with maplibre-gl library #4650

Open
kartikgoyal04 opened this issue Sep 3, 2024 · 3 comments
Open

Memory Leak while interacting with maplibre-gl library #4650

kartikgoyal04 opened this issue Sep 3, 2024 · 3 comments
Labels
bug Something isn't working good first issue Good for newcomers PR is more than welcomed Extra attention is needed

Comments

@kartikgoyal04
Copy link

kartikgoyal04 commented Sep 3, 2024

maplibre-gl-js version: 4.5.0

browser: Chrome

Steps to Trigger Behavior

  1. Add source, layer and markers atleast 10 times per second like below:
export const renderIcon = (
map: maplibregl.Map,
Position: Position,
missionVolumes: MissionVolumes,
OrVolumesClicked: (missionVolumes: MissionVolumes) => void,
) => {
const LayerId = `-${Position.Id}`;
const Source = map.getSource(LayerId) as any;

const clickHandler = handleClick(map, Position.Id, missionVolumes, OrVolumesClicked);

const renderDetails = (root: Root, Position: Position, missionVolumes: MissionVolumes) => {
  root.render(
    <Details
      ={{
        id: Position.Id,
        heading: Position.rotationDegrees,
        altitude: Position.altitude,
        horizontalSpeed: Position.horizontalSpeed,
      }}
      missionVolumes={missionVolumes}
      onClick={clickHandler}
    />,
  );
};

// Initialize marker if it doesn't exist
if (!markersMap.has(Position.Id)) {
  if (!Source) {
    map.addSource(LayerId, {
      type: "geojson",
      data: setMapData(Position.position),
    });

    map.addLayer({
      id: LayerId,
      type: "symbol",
      source: `-${Position.Id}`,
      layout: {
        "icon-image": MapIconId.Icon,
        "icon-size": 0.35,
        "icon-rotate": Position.rotationDegrees,
        "icon-allow-overlap": true,
      },
    });
  }

  const el = document.createElement("div");
  const root = createRoot(el);
  // TODO: Uncomment later
  // renderDetails(root, Position, missionVolumes);

  const marker = new maplibregl.Marker({ element: el, offset: [0, 55] }).setLngLat(Position.position).addTo(map);

  markersMap.set(Position.Id, { marker, root });
} else {
  const markerData = markersMap.get(Position.Id);
  if (markerData) {
    const { marker, root } = markerData;
    // Update the marker position without removing it
    marker.setLngLat(Position.position);

    // Update the  details by re-rendering the React component
    // TODO: Uncomment later
    // renderDetails(root, Position, missionVolumes);
  }

  if (Source) {
    const sourceData = (Source as any).serialize().data;
    const startPosition =
      sourceData && sourceData.features && sourceData.features[0]
        ? sourceData.features[0].geometry.coordinates
        : Position.position;

    const startTime = performance.now();
    const durationMs = 1000;

    const animateMarker = (timestamp: number) => {
      const elapsedTime = timestamp - startTime;
      const t = Math.min(1, elapsedTime / durationMs);

      const interpolatedPosition = [
        startPosition[0] + (Position.position[0] - startPosition[0]) * t,
        startPosition[1] + (Position.position[1] - startPosition[1]) * t,
      ] as [number, number];

      Source.setData(setMapData(interpolatedPosition));

      const markerData = markersMap.get(Position.Id);
      if (markerData) {
        const { marker } = markerData;
        marker.setLngLat(interpolatedPosition);
      }

      if (t < 1) requestAnimationFrame(animateMarker);
    };

    requestAnimationFrame(animateMarker);
  }
}

map.setLayoutProperty(LayerId, "icon-rotate", Position.rotationDegrees);
};

After taking the heapdump via chrome debug tools, I found that there are AbortControllers object holding the memory and not getting garbage collected. Here is the code of maplibre-gl library that instantiating these objects :

/**
 * An implementation of the [Actor design pattern](https://en.wikipedia.org/wiki/Actor_model)
 * that maintains the relationship between asynchronous tasks and the objects
 * that spin them off - in this case, tasks like parsing parts of styles,
 * owned by the styles
 */
class Actor {
    /**
     * @param target - The target
     * @param mapId - A unique identifier for the Map instance using this Actor.
     */
    constructor(target, mapId) {
        this.target = target;
        this.mapId = mapId;
        this.resolveRejects = {};
        this.tasks = {};
        this.taskQueue = [];
        this.abortControllers = {};
        this.messageHandlers = {};
        this.invoker = new ThrottledInvoker(() => this.process());
        this.subscription = subscribe(this.target, 'message', (message) => this.receive(message), false);
        this.globalScope = isWorker(self) ? target : window;
    }
    registerMessageHandler(type, handler) {
        this.messageHandlers[type] = handler;
    }
    /**
     * Sends a message from a main-thread map to a Worker or from a Worker back to
     * a main-thread map instance.
     * @param message - the message to send
     * @param abortController - an optional AbortController to abort the request
     * @returns a promise that will be resolved with the response data
     */
    sendAsync(message, abortController) {
        return new Promise((resolve, reject) => {
            // We're using a string ID instead of numbers because they are being used as object keys
            // anyway, and thus stringified implicitly. We use random IDs because an actor may receive
            // message from multiple other actors which could run in different execution context. A
            // linearly increasing ID could produce collisions.
            const id = Math.round((Math.random() * 1e18)).toString(36).substring(0, 10);
            this.resolveRejects[id] = {
                resolve,
                reject
            };
            if (abortController) {
                abortController.signal.addEventListener('abort', () => {
                    delete this.resolveRejects[id];
                    const cancelMessage = {
                        id,
                        type: '<cancel>',
                        origin: location.origin,
                        targetMapId: message.targetMapId,
                        sourceMapId: this.mapId
                    };
                    this.target.postMessage(cancelMessage);
                    // In case of abort the current behavior is to keep the promise pending.
                }, { once: true });
            }
            const buffers = [];
            const messageToPost = Object.assign(Object.assign({}, message), { id, sourceMapId: this.mapId, origin: location.origin, data: serialize(message.data, buffers) });
            this.target.postMessage(messageToPost, { transfer: buffers });
        });
    }
    receive(message) {
        const data = message.data;
        const id = data.id;
        if (data.origin !== 'file://' && location.origin !== 'file://' && data.origin !== location.origin) {
            return;
        }
        if (data.targetMapId && this.mapId !== data.targetMapId) {
            return;
        }
        if (data.type === '<cancel>') {
            // Remove the original request from the queue. This is only possible if it
            // hasn't been kicked off yet. The id will remain in the queue, but because
            // there is no associated task, it will be dropped once it's time to execute it.
            delete this.tasks[id];
            const abortController = this.abortControllers[id];
            delete this.abortControllers[id];
            if (abortController) {
                abortController.abort();
            }
            
            
            return;
        }
        if (isWorker(self) || data.mustQueue) {
            // In workers, store the tasks that we need to process before actually processing them. This
            // is necessary because we want to keep receiving messages, and in particular,
            // <cancel> messages. Some tasks may take a while in the worker thread, so before
            // executing the next task in our queue, postMessage preempts this and <cancel>
            // messages can be processed. We're using a MessageChannel object to get throttle the
            // process() flow to one at a time.
            this.tasks[id] = data;
            this.taskQueue.push(id);
            this.invoker.trigger();
            return;
        }
        // In the main thread, process messages immediately so that other work does not slip in
        // between getting partial data back from workers.
        this.processTask(id, data);
    }
    process() {
        if (this.taskQueue.length === 0) {
            return;
        }
        const id = this.taskQueue.shift();
        const task = this.tasks[id];
        delete this.tasks[id];
        // Schedule another process call if we know there's more to process _before_ invoking the
        // current task. This is necessary so that processing continues even if the current task
        // doesn't execute successfully.
        if (this.taskQueue.length > 0) {
            this.invoker.trigger();
        }
        if (!task) {
            // If the task ID doesn't have associated task data anymore, it was canceled.
            return;
        }
        this.processTask(id, task);
    }
    processTask(id, task) {
        return __awaiter(this, void 0, void 0, function* () {
            if (task.type === '<response>') {
                // The `completeTask` function in the counterpart actor has been called, and we are now
                // resolving or rejecting the promise in the originating actor, if there is one.
                const resolveReject = this.resolveRejects[id];
                delete this.resolveRejects[id];
                if (!resolveReject) {
                    // If we get a response, but don't have a resolve or reject, the request was canceled.
                    return;
                }
                if (task.error) {
                    resolveReject.reject(deserialize(task.error));
                }
                else {
                    resolveReject.resolve(deserialize(task.data));
                }
                return;
            }
            if (!this.messageHandlers[task.type]) {
                this.completeTask(id, new Error(`Could not find a registered handler for ${task.type}, map ID: ${this.mapId}, available handlers: ${Object.keys(this.messageHandlers).join(', ')}`));
                return;
            }
            const params = deserialize(task.data);
            const abortController = new AbortController();
            this.abortControllers[id] = abortController;
            try {
                const data = yield this.messageHandlers[task.type](task.sourceMapId, params, abortController);
                this.completeTask(id, null, data);
            }
            catch (err) {
                this.completeTask(id, err);
            }
        });
    }
    completeTask(id, err, data) {
        const buffers = [];
        delete this.abortControllers[id];
        const responseMessage = {
            id,
            type: '<response>',
            sourceMapId: this.mapId,
            origin: location.origin,
            error: err ? serialize(err) : null,
            data: serialize(data, buffers)
        };
        this.target.postMessage(responseMessage, { transfer: buffers });
    }
    remove() {
        this.invoker.remove();
        this.subscription.unsubscribe();
    }
}

Link to Demonstration

Expected Behavior

Objects should get garbage collected and heap memory should not increase overtime.

Actual Behavior

We are seeing increase in memory usage overtime. Sharing the snapshot of objects that are not garbage collected.

@kartikgoyal04
Copy link
Author


image-2024-08-21-15-06-15-039
image-2024-08-21-15-50-47-708

@HarelM
Copy link
Collaborator

HarelM commented Sep 3, 2024

Probably a path where the abort controller is not getting deleted (happy path I guess).
Feel free to submit a PR to solve this.
Looks like the code in actor.ts, which is new to version 4.

@HarelM HarelM added bug Something isn't working PR is more than welcomed Extra attention is needed good first issue Good for newcomers labels Sep 3, 2024
@kartikgoyal04
Copy link
Author

Reducing maplibre version to 3.6.2 solved the memory leak issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers PR is more than welcomed Extra attention is needed
Projects
None yet
Development

No branches or pull requests

2 participants