Modify

Opened 4 years ago

Closed 3 months ago

Last modified 3 months ago

#13805 closed enhancement (fixed)

improve progress bar of validator

Reported by: Klumbumbus Owned by: team
Priority: normal Milestone: 20.03
Component: Core validator Version:
Keywords: template_report Cc:

Description (last modified by Klumbumbus)

  1. validate a larger data set
  2. see how the test "Tag checker (MapCSS based)" takes by far the longest time of all the test
  3. during this test you see no progess in the progress bar
  4. for better user feedback it would be nice to print all the single mapcss based tests separately
  5. --> more "fluent" validation progress

This would count for all core tests and all active Rules


Build-Date:2016-10-14 21:42:43
Revision:11128
Is-Local-Build:true

Identification: JOSM/1.5 (11128 SVN en) Windows 7 32-Bit
Memory Usage: 247 MB / 247 MB (0 MB allocated, but free)
Java version: 1.8.0_102-b14, Oracle Corporation, Java HotSpot(TM) Client VM
Screen: \Display0 1680x1050
Maximum Screen Size: 1680x1050
Dataset consistency test: No problems found

Attachments (3)

tagcheckerprogress.png (33.9 KB) - added by Klumbumbus 4 years ago.
13805-grouped.patch (6.8 KB) - added by GerdP 3 months ago.
13805-grouped.2.patch (8.4 KB) - added by GerdP 3 months ago.

Download all attachments as: .zip

Change History (23)

Changed 4 years ago by Klumbumbus

Attachment: tagcheckerprogress.png added

comment:1 Changed 4 years ago by Klumbumbus

Description: modified (diff)

comment:2 Changed 21 months ago by anonymous

Would be great if implemented.

comment:3 Changed 3 months ago by GerdP

Resolution: duplicate
Status: newclosed

Closed as duplicate of #17035.

comment:4 Changed 3 months ago by Klumbumbus

Not really fixed. There is a counter and progress now within "Tag checker (MapCSS based)", however the single rules (geometry, numeric, germany specific,... ) are not printed. That may be too complex/too much work?

comment:5 Changed 3 months ago by GerdP

The algo works like this:
For each object an index is used to find which tests may match. For each processed primitive the counter is increased.
This is typically done in miliseconds. It would not help much to show several strings in that time, it would just slow down the test.

comment:6 Changed 3 months ago by GerdP

On the other hand it might well be possible to change the logic so that each *.mapcss file is processed as a single test.
Maybe it was always expected to work like that? See also #17737.

Last edited 3 months ago by GerdP (previous) (diff)

comment:7 in reply to:  6 ; Changed 3 months ago by Klumbumbus

Replying to GerdP:

On the other hand it might well be possible to change the logic so that each *.mapcss file is processed as a single test.

If that makes the validation slower then we shouldn't change it. Imho performance is more important than displaying the single rules.

comment:8 in reply to:  7 Changed 3 months ago by GerdP

Replying to Klumbumbus:

Replying to GerdP:

On the other hand it might well be possible to change the logic so that each *.mapcss file is processed as a single test.

If that makes the validation slower then we shouldn't change it. Imho performance is more important than displaying the single rules.

I don't think it would change performance to show "geometry" or "deprecated" if we change the logic so that we have multiple instances of MapCSSTagChecker, one for each *.mapcss file. It clearly would be the solution for #17737.

comment:9 Changed 3 months ago by GerdP

The current code in OsmValidator and ValidateAction doesn't make it easy to change this. We probably have to treat
test MapCSSTagChecker completely separated from the other tests. Up to now we use several loops like

for (Test test : allTests) {
    test.foo();
}

I don't like the idea to add a special case for each of the loops and I don't see how to avoid that either. Maybe there are tricks like reflection to do this but I don't know them.

comment:10 Changed 3 months ago by Klumbumbus

OK, then let's keep the current state. Thanks for looking into this.

comment:11 Changed 3 months ago by GerdP

Changed my mind. It's probably easier than I thought.

Changed 3 months ago by GerdP

Attachment: 13805-grouped.patch added

comment:12 Changed 3 months ago by GerdP

13805-grouped.patch:

  • change flow control so that the tests of one *.mapcss file are performed as a block without side effects on other tests (#17737)
  • show the source name (e.g. geometry.mapcss) in the progess bar and an element counter if the tests runs longer than 0.5 seconds
  • element counter is increased in steps of 10000 instesd of 1000. Not sure if that is better.

I saw no changes regarding performance. Waiting for #18802 to be closed...

comment:13 Changed 3 months ago by Klumbumbus

The patch works fine for me. I would prefer the translated names instead of the file names though if possible:

right part of:


Last edited 3 months ago by Klumbumbus (previous) (diff)

comment:14 in reply to:  13 Changed 3 months ago by GerdP

Replying to Klumbumbus:

The patch works fine for me. I would prefer the translated names instead of the file names though if possible:

Thanks for testing. Problem regarding translated name is the same as with the tool tip (#18810), so I have two good reasons to work on this ;)

Changed 3 months ago by GerdP

Attachment: 13805-grouped.2.patch added

comment:15 Changed 3 months ago by GerdP

The new patch shows the translated name or the name the user has given for his own *.mapcss file. If that optional name is empty it shows tr("unknown"). Altenative would be to show the file name instead as with the first patch version.

comment:16 Changed 3 months ago by Klumbumbus

Resolution: duplicate
Status: closedreopened

I'll test after ticket:18845#comment:13 is fixed.

comment:17 Changed 3 months ago by Klumbumbus

13805-grouped.2.patch​ works fine for me. For the fallback for missing name I would use the filename then as "unknown" gives pretty much no informationen.

comment:18 Changed 3 months ago by Klumbumbus

Summary: improve progess bar of validatorimprove progress bar of validator

comment:19 Changed 3 months ago by GerdP

Resolution: fixed
Status: reopenedclosed

In 16047/josm:

fix #13805,#17737 improve progress bar of validator / Unclear scope of mapcss class (set clause)

  • change flow control so that the tests of one *.mapcss file are performed as a block without side effects on other tests (#17737). Tests which relied on this might need changes.
  • show the rules name (e.g. Geometry) in the progess bar and an element counter if the tests runs longer than 0.5 seconds. If no name was given for the rules the file name is shown (e.g. myrules.mapcss)
  • element counter is increased in steps of 10000 instead of 1000.

comment:20 Changed 3 months ago by Klumbumbus

Milestone: 20.03

Perfect, thanks!

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain team.
as The resolution will be set.
The resolution will be deleted.

Add Comment


E-mail address and name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.