Table view cell enhancement!

[vincentneo]

Revamped the table view cells! Added distance travelled and first location of tracked in the gpx file! I always wanted that feature. What do you think? Kinda related to #14, ~~but I could not implement track duration yet.~~ added in commit https://github.com/merlos/iOS-Open-GPX-Tracker/pull/63/commits/1d9bb5e1e26c2678e0931222065e98f534ae5263

By the way, I feel that it could be good if the overall interface can be revamped a little? Like for a version 2 or something? If you liked it, maybe I could do something for this app, as I would be free for the coming weeks.

Having benefitted from this app for a long time, I really feel like doing something for it. 😊

[merlos]

Thanks Vincent.

Increasing the information of about the files is interesting. Will have to review the changes. In particular there are two things that I am concerned about:

1. The use of Interface Builder. My main concern is for consistency and standards on the code. The rest of the UI is done programmatically.
2. How the first location tracked reverse geocoding performs. It is done each time the list is loaded, and I assume it uses internet. Did you think about using the geocoding in creation time linked to the name? Adn did you tested it :
   • With a large number of files?
   • With slow Internet connections?
   • With no Internet connection?
3. Files are parsed twice: 1 to get the file size and 1 to get the first location. Does it perform well when you have large number of files that weight a few MB? I see that information more related with adding some metadata to the file because it is always the same on saving time.

Regarding the interface revamp. What are you thinking about? There are two things that I am particularly interested about but are not related with the UI:

1. Is porting GPX framework to swift (now it is in Objective-C). adding better support of GPX extensions (For example it would allow to add more data to each point such as heading, speed)

2. Is improving loading times of tiles in the map of alternative sources (pre fetching them) and improving how cache works offline. In fact, I think that a specialized map caching and offline could be an interesting project itself. Last time I checked there were no many good map cache projects.

BTW, sorry that I still have to review your other two pull requests.

Best, Juan.

[vincentneo]

Hey Juan,

Indeed some very important questions about this pull request. I did not even think about these issues before that. I will do some debugging probably next week and test all the variables listed by you. Maybe I'll skip using the interface builder all together if I could.

Regarding the stuff you're particularly interested towards, I am also personally very interested in porting the GPX framework to Swift, but it would probably be quite a problem for me, as my literacy towards Objective C is kinda bad... Maybe I'll try though, I always treated these as learning opportunities (I am still a student)

As for the map caching thing, it does sound very interesting. Though how would such a system work? Maybe I'll do some research, sounds fun.

Thats it for now, from me. Do take your time for reviewing my pull requests btw.

[vincentneo]

Alright, cell UI is now done fully programmatically. Will debug next time. Thats all for today.

Regards, Vincent

[vincentneo]

Hello!

Been a while since my last update. As you probably know, I was working hard on the iOS-GPX-Framework port (GPXKit). Fully ported now, but as I already expected from the start of the project, tons of inconsistencies, misunderstandings of code and bugs will slow progress down.

Meanwhile, may I request for you to review my code anytime when you're free, probably in 2019, as I feel that it probably has a ton of misdirections from my lack of understandings in both languages (like when to use protocols, access control stuff and init). Just like a guidance kind of thing? Please? 😊

In the meantime, happy holidays!

Sincerely, Vincent

[merlos]

That´s awesome @vincentneo! I took a quick look and it looks pretty good... but, sure, I´ll go through it more deeply. Did you think in getting rid of the other Objective-C dependency for parsing the XML? I guess current Apple´s library is much better now than it used to be.

As I told you, one of the things i am specially interested in is the possibility of saving more info in the GPX file and using extensions. For example, to store the metadata, such as the location as you propose in this pull request, or on each point to add more info such as the speed.

Best, Juan!

[vincentneo]

Firstly, thanks for taking a look at it! Gives me a bit of a assurance towards the goal of a stable port.

Right now, as there lies critical issues, I will have to fix them before adding the new stuff, like storing metadata and such.

The native XML library from Apple does worth taking a look, as it does seem a little weird to still rely on a Objective C dependency. The fact that the problem I am facing right now is also partly from that dependency, might make me switch to the native one too.

Hope my next comment would be one that brings some good news here!

Sincerely, Vincent

[vincentneo]

I have researched more on the native XML parser, and its said to be a SAX parser that is supposingly slower than TBXML which is a DOM parser. What are your thoughts on this?

[merlos]

I think that if you were working directly with the XML file, for instance, adding, searching, removing updating XML elements, it could be argued that there may be big differences when using DOM as it is already in memory... however, for just loading the XML into a model or dumping the model as XML... I don´t see performance should be an issue... even more, as far as I know, a SAX parser should be faster...

[vincentneo]

Alright. I'll see how things goes!

[vincentneo]

Hey Juan!

I've finished up GPXKit! generates and parses without problems using the native XMLParser for parsing GPX files, based on my preliminary testing. I have yet to submit it to CocoaPods due to a naming conflict. ~~(There apparently is already a GPXKit somewhere else) Probably going to rename it. (any ideas for alternative ways?)~~ project renamed to CoreGPX

~~After I sort that out, I will start adding features and maybe do a pull request for this project. Btw, it would be great if you could go through a list of features that you wished for the port. ~~

Found a date bug. Oops... Important stuff. Priority is that now.

Hope to hear back soon!

[merlos]

Awesome!

Outside is -6ºC, so let me see if I can get some time this weekend...

Names, well, it is up to you as creator, but as ideas: GPXLib - Classic name of a library GPX-Swift - Better for SEO

[vincentneo]

Oops... I named it CoreGPX as like CoreLocation naming... I named it earlier this afternoon, and it was a huge hassle. Not going to change it again sorry 😅

Btw, -6 degrees?! Damn that must be cold. Its always around 30 degrees here.

As I said in my edited comment, the parsing of the GPX file always return the time of parsing, not what is shown on the file... Quite problematic I'm guessing...

[merlos]

Hi @vincentneo,

Don´t worry about the name, it is your project, so you decide :D.

I created a new branch to test Core GPX on the app. https://github.com/merlos/iOS-Open-GPX-Tracker/tree/features/move-to-coregpx

Changes to make it work were very straight forward, that was pretty cool.

Till now I found a couple of issues on the integration:

1. Some files display a line that joins the first and last point.
2. Large files, >1MB take a lot to load or kill the app.

(I haven´t researched yet the causes)

[vincentneo]

I do have a forked branch as well, https://github.com/vincentneo/iOS-Open-GPX-Tracker/tree/GPXKit This branch was used for the testing of the library’s code during its development.

Back then, I did notice that large files took a long time to load, hence I didn’t do a pull request yet. I didn’t expect that it crashes too. (Got to take a look at that real soon)

As for the line that joins the first and last, I didn’t observe it (at least in my fork) will test it on your branch to see if that happens, would also test with a wider range of files.

[merlos]

Hi @vincentneo.

I continued testing the issues, but no luck till now :(

For long files, XML parsing takes a lot of time... I don´t know yet, but if the main reason is because of the SAX parser, definitely, it was SLOW and TBXML was the way to go. It is super weird, but 2.5 MB file takes ~1 min to parse in my computer...

I committed a couple of changes in this branch, https://github.com/merlos/iOS-Open-GPX-Tracker/tree/features/move-to-coregpx-tests but it is still buggy. When trk element ends, on my code, it does not clean the track.tracksegments variable properly, so it accumulates the tracks (I guess that´s the reason of the contains on the add of track, but there shall be any other way...).

To optimize the performance, I moved the most common at the beginning of the switch...

[vincentneo]

On my testing, a 1.3mb file can take around 15-20s on the iPhone X compared to milliseconds in the App Store Version.

Hmm could it be because I was using switch instead of if/else and the XML parser is recursively calling on the empty default?

I mean the SAX parser shouldn’t be THAT slow right? There must be something wrong?

[merlos]

For me it is really weird it takes that time, 20 seconds in an iPhone X is a lot of computation time for only 1.3MB.

My guesses are:

• Objects being copied many times (for instance append/.add(),etc)
• Switch vs if/else

But yesterday I could not get to any conclusion...

https://github.com/apple/swift/blob/master/docs/OptimizationTips.rst

[vincentneo]

Ok. I’ll see if I have the time this week to work on this... I’ll try a few variables of the code.

Btw, I briefly remembered that the CPU usage on parsing is quite high as well.

[merlos]

Yes, CPU usage is at 100%. However, memory remains pretty stable, which may dismiss the copy theory.

Maybe string comparisons/processing is related with that heavy CPU usage.

[vincentneo]

Another interesting tidbit: I was testing this ancient 4.5MB GPX file created by another app, and it definitely took a long time (1min 33s), but also a interesting bug where the total kilometer is different. The app store one shows 116.39km but the Core GPX one shows 115.66km

Could some points be lost in between parsing? I don't have my mac accessible so everythings done on my iPhone X.

[vincentneo]

https://user-images.githubusercontent.com/23420208/51126929-492a9d00-185f-11e9-8884-44bbd65256a3.PNG (Core GPX)

https://user-images.githubusercontent.com/23420208/51126930-49c33380-185f-11e9-9980-7fc905f20134.PNG (App Store Version) (View them side by side)

Both are on max zoom btw.

I think I can confirm that points were missing/rounded off with those two images. Same file, different stories.

[vincentneo]

Was debugging for the entire morning, and I think I might have reduced the speed issue quite a bit.

I didn't use the this app for debugging, as I used the example app for the Core GPX to do the testing.

Using Xcode instruments, I was able to find that this code was very CPU intensive, particularly the contains check.

        if trackpoint != nil {
            let contains = trackpoints.contains(trackpoint!)
            if contains == false {
                trackpoint?.parent = self
                trackpoints.append(trackpoint!)
            }
        }
    }

Previously, the add(trackpoints:) method used a for loop that calls on the above code. The contains check then slows everything down.

With the same file from yesterday (4.5MB), parsing time in the example app dropped from 3 mins 40s to 17s.

While its still slow, there is a huge improvement.

Now the question is, do you know whether if the contains check was actually necessary? I had that because the original iOS-GPX-framework had this:

- (void)addTrackpoint:(GPXTrackPoint *)trackpoint
{
    if (trackpoint) {
        NSUInteger index = [_trackpoints indexOfObject:trackpoint];
        if (index == NSNotFound) {
            trackpoint.parent = self;
            [_trackpoints addObject:trackpoint];
        }
    }
}

PS: I have included two instrument files. The old one refers to testing with the contains check. CoreGPX_CPU_new.trace.zip CoreGPX_CPU_old.trace.zip

[merlos]

Huge improvement!

1. Distance calculation error. Thanks, I opened a bug https://github.com/merlos/iOS-Open-GPX-Tracker/issues/68. I noticed that once, but never researched further...

2. Contains check when adding a point. When I saw that in your code I wondered the need too, in my tests I removed it. I did not understand the reason why it was there... My only guess is that some GPS devices may record the same position many times if user is static at the same location... but the timestamp would be different.

3. One additional improvement may be on GPXParser.swift to move the trackpoints, waypoints and route points to the top of the switch statements. These elements are the most common to parse, so it may reduce the number of string comparisons to process.

[vincentneo]

By the way, during my previous round of debugging, probably another source of slow down is from the conversion from String to Date and String to CGFloat.

I haven't extensively tested that though. Replacing and using If/else does not seem to have an impact during the last time I tested.

Will do more testing and debugging next week!

[vincentneo]

Testing without the conversions shows an improvement from 17s to around 3s on best case, same file tested. My testing would be slower as all tests are conducted in my example app, which seems to have reduced performance due to the tableview. (from the instrument app)

Do you know of any better/more efficient way of converting from string to which ever type?

[merlos]

Hi Vincent,

Sorry, What do you mean converting string to which ever type? It may depend on the type...where do you have to convert it?

What is the speed if you get the table view out of the equation (as table view is not part of the library)?

[vincentneo]

By converting, I mean converting from String to CGFloat and Date, as the parser gives everything in String, while types like waypoints having things like latitude or elevation stored as CGFloat and time as Date.

I have not tested the speed without the table view, I only remembered observing a significant percentage reported by the instruments app that seems to be related to the table view.

[vincentneo]

Hi Juan,

I have tested the Open GPX Tracker with CoreGPX branch 'SpeedOptimisation' and the time taken to parse the 4.5MB file is now approximately 2.1s on the iPhone X, according Instruments's Time Profiler.

Do try this out: ~https://github.com/vincentneo/iOS-Open-GPX-Tracker/tree/GPXKit-Optimisation~

~By the way, I am thinking of switching all CGFloat types to Double. What do you think about this?~ This instead: https://github.com/vincentneo/iOS-Open-GPX-Tracker/tree/GPXKit-Optimisation%2BDouble

[merlos]

As I understand CGFloat is just a wrapper in swift to keep some kind of compatibility, but at the end you have a Double.

Location services, seem to be using Double (https://developer.apple.com/documentation/corelocation/cllocationdegrees) and no CGFloat so, probably we will save some type castings if you use Double.

public struct CGFloat {

    /// The native type used to store the CGFloat, which is Float on
    /// 32-bit architectures and Double on 64-bit architectures.
    public typealias NativeType = Double

    public init()

    /// Creates a new instance from the given value, rounded to the closest
    /// possible representation.
    ///
    /// - Parameter value: A floating-point value to be converted.
    public init(_ value: Float)

    /// Creates a new instance from the given value, rounded to the closest
    /// possible representation.
    ///
    /// - Parameter value: A floating-point value to be converted.
    public init(_ value: Double)

    /// Creates a new instance from the given value, rounded to the closest
    /// possible representation.
    ///
    /// - Parameter value: A floating-point value to be converted.
    public init(_ value: Float80)

    public init(_ value: CGFloat)

    /// Creates a new value, rounded to the closest possible representation.
    ///
    /// If two representable values are equally close, the result is the value
    /// with more trailing zeros in its significand bit pattern.
    ///
    /// - Parameter value: The integer to convert to a floating-point value.
    public init(_ value: UInt8)

    /// Creates a new value, rounded to the closest possible representation.
    ///
    /// If two representable values are equally close, the result is the value
    /// with more trailing zeros in its significand bit pattern.
    ///
    /// - Parameter value: The integer to convert to a floating-point value.
    public init(_ value: Int8)

    /// Creates a new value, rounded to the closest possible representation.
    ///
    /// If two representable values are equally close, the result is the value
    /// with more trailing zeros in its significand bit pattern.
    ///
    /// - Parameter value: The integer to convert to a floating-point value.
    public init(_ value: UInt16)

    /// Creates a new value, rounded to the closest possible representation.
    ///
    /// If two representable values are equally close, the result is the value
    /// with more trailing zeros in its significand bit pattern.
    ///
    /// - Parameter value: The integer to convert to a floating-point value.
    public init(_ value: Int16)

    /// Creates a new value, rounded to the closest possible representation.
    ///
    /// If two representable values are equally close, the result is the value
    /// with more trailing zeros in its significand bit pattern.
    ///
    /// - Parameter value: The integer to convert to a floating-point value.
    public init(_ value: UInt32)

    /// Creates a new value, rounded to the closest possible representation.
    ///
    /// If two representable values are equally close, the result is the value
    /// with more trailing zeros in its significand bit pattern.
    ///
    /// - Parameter value: The integer to convert to a floating-point value.
    public init(_ value: Int32)

    /// Creates a new value, rounded to the closest possible representation.
    ///
    /// If two representable values are equally close, the result is the value
    /// with more trailing zeros in its significand bit pattern.
    ///
    /// - Parameter value: The integer to convert to a floating-point value.
    public init(_ value: UInt64)

    /// Creates a new value, rounded to the closest possible representation.
    ///
    /// If two representable values are equally close, the result is the value
    /// with more trailing zeros in its significand bit pattern.
    ///
    /// - Parameter value: The integer to convert to a floating-point value.
    public init(_ value: Int64)

    /// Creates a new value, rounded to the closest possible representation.
    ///
    /// If two representable values are equally close, the result is the value
    /// with more trailing zeros in its significand bit pattern.
    ///
    /// - Parameter value: The integer to convert to a floating-point value.
    public init(_ value: UInt)

    /// Creates a new value, rounded to the closest possible representation.
    ///
    /// If two representable values are equally close, the result is the value
    /// with more trailing zeros in its significand bit pattern.
    ///
    /// - Parameter value: The integer to convert to a floating-point value.
    public init(_ value: Int)

    /// The native value.
    public var native: CGFloat.NativeType
}

[vincentneo]

@merlos ok, will be using Double instead of CGFloat