Skip to content

Various performance fixes #318

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

Merged
merged 6 commits into from
Jan 23, 2024
Merged

Various performance fixes #318

merged 6 commits into from
Jan 23, 2024

Conversation

isc-tleavitt
Copy link
Collaborator

Throwing this out there as a draft - more testing is needed (both cross-OS on Windows/Linux AND cross-version with/without /ENV on $zf(-100)) before saying it's good to go.

This address both #315 and #269, as well as some issues I've seen with lag in the WebUI, by the following measures:

  • Caching the InternalName/ExternalName (that is, in the DB + on the filesystem) mapping
  • Caching results of %Library.RoutineMgr:UserType
  • Removing needless calls to %File:NormalizeFilename (which is surprisingly slow)
  • Removing /SHELL from $zf(-100) calls as much as possible
  • Reducing WebUI calls to RefreshUncommitted and in general putting a 10-second throttle on refreshes of the uncommitted queue (which may be bypassed for things like e.g. commit)

Outcomes for a larger project are:

  • Opening a Studio tab (or 10) is much faster
  • The WebUI goes from laggy to virtually instantaneous, most notably for the initial load but also with a significant improvement when actually viewing uncommitted changes

Testing should cover:

  • Ensuring no regressions with the old issue trying to use root's global attributes on Linux (without /SHELL, covered using $XDG_CONFIG_HOME with /ENV as described at https://git-scm.com/docs/git-config#FILES) - need to test on versions without /ENV support in $zf(-100)
  • General git operations on Windows/Linux (unit tests are actually pretty representative here for the Linux side, and I've run locally on Windows)

* Throttle and cache RefreshUncommitted
* Drop /SHELL in commands as much as possible (good for non-performance reasons too)
Avoid calling NormalizeFilename (shouldn't ever actually be necessary in cases it was used) as it's highly expensive
Reduce calls to %RoutineMgr:UserType (TODO: caching layer around this would help too; this is the main bottleneck other than $zf(-100) calls)
Cache InternalName/ExternalName mapping in a new table (reduces overhead of $System.OBJ.Load calls)
Unlock session for WebUI calls to avoid bottlenecks and make UI more snappy
RefreshUncommitted on fewer WebUI git calls
@isc-pbarton
Copy link
Collaborator

Something I found testing this on Unix with an old IRIS version (2019.1). Setting up git-source-control with Configure() fails:
ERROR #5002: ObjectScript error: <METHOD DOES NOT EXIST>zOnAfterConfigure+39^SourceControl.Git.Settings.1 *Sync,%SYSTEM.WorkMgr
I think this is a regression caused by #255. The Sync method was added in IRIS 2021.2, which is after the /ENV flag was added to $zf(-100).

@isc-pbarton
Copy link
Collaborator

Actually looks like that should be pretty quick to fix. Even though it's unrelated it might be easiest to tack onto this PR so we can test the case with Unix before /ENV support. What do you think @isc-tleavitt? I could work on it next week if you'd like.

@isc-tleavitt
Copy link
Collaborator Author

@isc-pbarton sorry for the slow response - if you can look into that and tack on a fix it'd be great

Copy link
Collaborator

@isc-pbarton isc-pbarton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tacked on a fix to the unrelated issue on older IRIS versions, and smoke-tested on Unix with IRIS 2019.1 and Windows with IRIS 2023.2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants