Performance issues when clearing the chart several times (ref: `hotline` and `almostOver` layers)

[mttucl]

Hi @Raruto,

I have a list of tracks and the option to add more tracks from geojsons or strings. What would be the proper way to clear a showing track and load a new one?

I made a JSFiddle example of my attempt but the issue is the more I switch between tracks, the slower the map becomes.

In your example, you add the layer and then addData instead of load the url or a string like in my case. I'm not sure if this is the issue but for my use, it is more convenient to load the json which will take care of the chart, and the layer e.g. hotline, and start/end markers.

I appreciate your help with this.

[Raruto]

Hi @mttucl

the issue is the more I switch between tracks, the slower the map becomes.

will take care of the chart, and the layer e.g. hotline, and start/end markers.

As you say, the problem could be related precisely to the fact that not all other support layers / markers are correctly removed from the map.

Keep in mind that for a long time my average use case of one layer per map (and per page!) was more than enough, that's why your case it's not much tested..

Waiting for improvement pull requests 👋 Raruto

[mttucl]

The issue seems to be with almostOver. No issue if you turn it off in the above JSFiddle example. It might have to do with almostOver plugin but mainly with _initAlmostOverHandler by registering below events every time it is called with load.

map
  .on('almost:move', (e) => this._onMouseMoveLayer(e))
  .on('almost:out',  (e) => this._onMouseOut(e));

Instead, it should handle any update with map.almostOver.removeLayer(layer) and map.almostOver.addLayer(layer).

I also see a performance issue with hotline but not a severe as almostOver. Turning both off gives the best performance which is a shame because I like them.

Sorry, I'm not very good at this and I don't have a proper fix.

My hack is with L.Control.Elevation.include by adding reload and _updateMapIntegrations that is similar to load and _initMapIntegrations but with calling below instead of _initAlmostOverHandler

this.on('eledata_clear', () => {
  map.almostOver.removeLayer(layer);
});

_updateAlmostOverHandler: function(map, layer) {
  if (L.GeometryUtil && map.almostOver && map.almostOver.enabled()) {
    map.almostOver.addLayer(layer);                                  
  }
}

I still see a performance issue with the above but it is much better than before.

[Raruto]

Hi @mttucl this is exactly one of those cases where it's worth writing some automated test (as you see from the /spec folder there is nothing meaningful about it..)

Since it's something intrinsic to this library I suggest you directly edit the code within the /src folder (during your tests), so then it becomes easy for you to propose a pull request.

As for the performance issue you notice some options (eg. the almostOver layer) this happens because these integrations need to add dummy layer (ie. opacity = 0) to the map in order to work properly, that's why at the moment they may not all be removed.

Here too, to facilitate the writing of the tests, it should be enough for you to check the value returned by the following property from time to time:

Object.keys(map._layers).length

Just to be clear, I know that without a starting point it can seem complicated or challenging:

• I will not write these tests for you..
• feel free to ask if you need additional information about the current code

Have a good debugging sessions 👋 Raruto

[mttucl]

Hi @Raruto,

I will be happy to contribute to this project. Fair warning, I'm a novice in using GitHub and have limited experience in Javascript. Is the test supposed to be in /spec folder to run Object.keys(map._layers).length when editing the code within the /src folder? What is it supposed to do? Just to show if the dummy layers are cleared every time new data is loaded? How is it automated? I looked up automated tests and I see mentions of GitHub Actions which I have no idea about but seems intersting.

In any case, I will keep debugging as I'm enjoying it and I would love to improve it.

[Raruto]

Hi @mttucl,

I will be happy to contribute to this project. Fair warning, I'm a novice in using GitHub and have limited experience in Javascript.

No problem, I've always kept these repositories active just to learn new things. But keep in mind my real use case is really much more mundane than anything I've published over the years, so I'm happy if someone else tries to lighten some of my work.

Is the test supposed to be in /spec folder to run Object.keys(map._layers).length when editing the code within the /src folder? What is it supposed to do? Just to show if the dummy layers are cleared every time new data is loaded? How is it automated? I looked up automated tests and I see mentions of GitHub Actions which I have no idea about but seems interesting.

Frontend testing is a tricky business:

• has high maintenance costs
• in order to design a fronted test you to learn / experiment various related projects
• frontend tests tends to be much slower to run
• sometimes if a test fails it's not so easy to figure out what to do
• ...

To give you an idea of the available ones, I report below some images from the article: Restructuring Frontend Testing Pyramid: alternative to Unit/Integration/E2E approach

NB

• at bottom line = you can write many tests because they are cheap
• at top = you can write few tests because the cost is high

Frontend testing pyramid (in short)

Frontend testing pyramid (in a real world)

Anyway, my advice is to take inspiration from what others (much more expert than me) have done. The leaflet repository is always a good source because it's more probable that you will find more information on the net.

As you can see within this repository this subject it has never been digged that much, except for:

• some "obvious" manual tests (ref: /examples folder)
• some "simple" unit tests (ref: /spec folder)

I would just like to point out, even if it doesn't solve the problem of this issue, that in the next period it was my intention to introduce some more specific jsdoc comments (ref: Types without Typescript)

In a local branch I was working on better documenting the available types for the options and the various event listeners, but even here the issue is to try to find the right compromise on code readability...

The limit is your imagination 👋 Raruto

[Raruto]

PS just to understand the concept of "manual tests":

• every demo file uploaded in the /examples folder could be considered a "manual test" because you have to open all of them in your browser to personally check if something is wrong (if I'm not mistaken the leaflet repo has a /debug folder with similar tests)
• instead with an "automatic test" (e.g. if you run npm run test from the terminal) you will be able to run several tests at the same time and you will always be sure if something has failed (there is no discretion on the result, because in this case uvu tells you if something has gone wrong crooked)

[Raruto]

Since it's something intrinsic to this library I suggest you directly edit the code within the /src folder (during your tests), so then it becomes easy for you to propose a pull request.

Is the test supposed to be in /spec folder to run Object.keys(map._layers).length when editing the code within the /src folder?

I meant that for your local debugging I don't recommend going the way of overriding functions which I showed you in here: https://github.com/Raruto/leaflet-elevation/issues/231#issuecomment-1429963677

In this case, If you don't want to go crazy, it's much easier to locally edit the code of this library (which you can found in the /src folder)

[mttucl]

Hi @Raruto,

Thanks for the explanation and pointers.

I'm looking through the code and noticed you have a featureGroup for hotline that you use to clear its layers but not for almostOver. Would it make sense to do the same for almostOver. For updating layers, we can still use _initAlmostOverHandler but with a logic to skip below if already initialized.

map
   .on('almost:move', (e) => this._onMouseMoveLayer(e))
   .on('almost:out',  (e) => this._onMouseOut(e));

For hotline, can we add trkseg in the same hotline featureGroup or a new group that we can clear. This is the only dummy layer I see with opacity = 0 option. Not sure if there are any others.

[Raruto]

@mttucl giving us a quick look, I think you just need to do the reverse of what is written here (ie. I don't think there is a need to create a "grouping" layer).

https://github.com/Raruto/leaflet-elevation/blob/868179234d8e9b8f7beaea57d0e34d8bd79a2622/src/control.js#L408

Ref: makinacorpus/Leaflet.AlmostOver/src/leaflet.almostover.js#L81-L97

BTW, at the moment what I think doesn't really matter, the important thing (however you intend to edit the code) is that the number of levels currently loaded in the map will always prove us what is right or wrong:

// expected: Object.keys(map._layers).length == 7

controlElevation.load(...);

// expected: Object.keys(map._layers).length == 19

controlElevation.clear()

// excpected: Object.keys(map._layers).length) == 7 

/* test accepted ? */

Just to make it clear, if you start taking your jsfiddle and edit it one step at a time in order to load it as a new file in the /examples folder you are already on the right track to understand how to write a test for this case (then knowing how to code / transform it in an automated test is just a next step).

The closest thing I've done in the past is: examples/leaflet-elevation_multiple-maps.html (but as you can see it has never derived in an automated test ..)

👋 Raruto

[Raruto]

Hi @mttucl,

here https://github.com/Raruto/leaflet-elevation/commit/88cfa1119986ed4c059bf5db643722d9fc482d35 you can see a starting point on how start coding some "end to end" tests.

You can try the code of that pull as follows:

npm i
npm run test

For the first times, it could be better if you use playwright through the graphical interface:

npm i
npx playwright test --ui

For more info:

• https://playwright.dev/docs/writing-tests
• https://playwright.dev/docs/running-tests
• https://playwright.dev/docs/codegen-intro

While hoping someone else joins forces..

👋 Raruto

[Raruto]

And here it is a new dedicated example from v2.3.0: examples/leaflet-elevation_clear-button.html

Now, someone else improve the related test file..

https://github.com/Raruto/leaflet-elevation/blob/382451f4810ea26cc99fecabee91af36cf09dec0/examples/leaflet-elevation_clear-button.spec.js#L1-L44