Fix performance problem at login #464

benjamingeer · 2017-03-21T10:02:01Z

This fixes a performance problem caused by the use of MessageUtil.toSource in debugging output in ProjectsResponderV1. Also:

Add a configuration option app.triplestore.profile-queries that outputs SPARQL queries and the amount of time they take.
Change ResourcesResponderV1SpecContextData to load the expected book context response from a file in JSON format, rather than including it in the source code, to keep IntelliJ IDEA from running out of memory when compiling it. This makes it easier to run a profiler in IntelliJ.
Use a UUID instead of the current time when generating a session ID in Authenticator.authenticateCredentials() (Creation of session id on login #290).
Use Future.sequence instead of Await.result in PermissionsResponderV1.permissionsDataGetV1().
Use Vector and Seq instead of List in PermissionsResponderV1.
Move functions for reading and writing text files out of FakeTriplestore and into a utility class, FileUtil.
Reformat some code.

I'll document how to profile Knora in VisualVM using its IntelliJ IDEA plugin in another pull request.

Fixes #443.
Fixes #290.

…t in source code.

tobiasschweizer · 2017-03-23T10:14:16Z

first login immediately after having reloaded the data is still slower, but faster than before.

benjamingeer · 2017-03-23T10:21:10Z

Will this be good enough for your production server for now, or do we need to investigate further at the moment?

tobiasschweizer · 2017-03-23T10:52:20Z

webapi/src/main/scala/org/knora/webapi/Main.scala

@@ -29,15 +29,15 @@ object Main extends App with LiveCore with KnoraService  {
    //Kamon.start()

    /* Check and wait until all actors are running */
-    checkActorSystem
+    checkActorSystem()


Did you do that because the method has side-effects? http://docs.scala-lang.org/style/method-invocation.html

IntelliJ pointed it out as a style recommendation.

tobiasschweizer · 2017-03-23T10:53:09Z

webapi/src/main/scala/org/knora/webapi/Main.scala


    val arglist = args.toList

    if (arglist.contains("loadDemoData")) StartupFlags.loadDemoData send true
    if (arglist.contains("allowResetTriplestoreContentOperationOverHTTP")) StartupFlags.allowResetTriplestoreContentOperationOverHTTP send true

    /* Start the HTTP layer, allowing access */
-    startService
+    startService()


tobiasschweizer · 2017-03-23T10:55:43Z

webapi/src/main/scala/org/knora/webapi/responders/v1/PermissionsResponderV1.scala

-                            groupInfo <- (responderManager ? GroupInfoByIRIGetRequest(groupIri, None)).mapTo[GroupInfoResponseV1]
-                            res = (groupInfo.group_info.belongsToProject, groupIri)
-                        } yield res
-                        Await.result(resFuture, 1.seconds)


Did that blocking Await have any effect slowing down the application?

Not as far as I could tell.

tobiasschweizer · 2017-03-23T11:02:56Z

webapi/src/test/scala/org/knora/webapi/responders/v1/ResourcesResponderV1Spec.scala

-                case response: ResourceContextResponseV1 => compareResourceCompoundContextResponses(received = response, expected = ResourcesResponderV1SpecContextData.expectedBookResourceContextResponse)
+                case response: ResourceContextResponseV1 =>
+                    val responseAsJson = response.toJsValue
+                    assert(responseAsJson == ResourcesResponderV1SpecContextData.expectedBookResourceContextResponse, "book context response did not match")


If I understand correctly, the correct ordering of the dependent pages (according to their seqnum) is part of the query. So a 1:1 comparison is good.

I am just mentioning this because there are some tests in which the results have to be ordered according to the same pattern before they are compared (the ordering does not matter for the correctness of the results).

This test always assumed that the order was fixed.

Yes, that is absolutely correct. I justed wanted to make sure that this is what we want.

I guess so, since the SPARQL query has:

ORDER BY ?seqnum ?sourceObject ?isPreview

tobiasschweizer · 2017-03-23T11:08:28Z

Will this be good enough for your production server for now, or do we need to investigate further at the moment?

Do you have any idea what could explain that the first login takes about three times longer than any subsequent one?
Also if a do a second login with another user, it is much faster than the first one.

benjamingeer · 2017-03-23T11:09:43Z

I believe that the first login loads and caches data from the triplestore, which is reused in subsequent logins.

tobiasschweizer · 2017-03-23T11:10:40Z

I believe that the first login loads and caches data from the triplestore, which is reused in subsequent logins.

Sounds reasonable. So why couldn't we load that data in the cache when the application is started?

benjamingeer · 2017-03-23T11:11:46Z

So why couldn't we load that data in the cache when die application is started?

I guess we could. You'd have to ask @subotic, he wrote this code.

tobiasschweizer · 2017-03-23T11:12:40Z

Hey @subotic, could we cache data needed for authentication when webapi is started?

benjamingeer · 2017-03-23T11:18:53Z

In any case, I suspect that would require a non-trivial reworking of the code, so if we do it, it should be done in another pull request. Meanwhile, can we merge this one?

tobiasschweizer

mörtsch

tobiasschweizer · 2017-03-23T11:20:02Z

could you make an issue for the caching so we do not forget? thx.

benjamingeer · 2017-03-23T11:25:02Z

Opened #468.

subotic · 2017-03-23T11:25:42Z

Hey @subotic, could we cache data needed for authentication when webapi is started?

We probably could. This would require creating a UserProfileV1 for each registered user and storing it into the cache. But I don't think that this will change anything. When a user is authenticated, then the same thing is executed for the first and any subsequent user.

subotic · 2017-03-23T11:26:20Z

I will move my last comment to the new issue.

Benjamin Geer added 4 commits March 15, 2017 17:19

enhancement (webapi): Add profiling to investigate #443.

650ea07

style (webapi): Add missing method call parentheses.

e418f2c

test: Read large expected response from a file instead of including i…

0a239e2

…t in source code.

fix (webapi): Fix performance problem in ProjectsResponderV1 (#443)

7719733

benjamingeer added the bug something isn't working label Mar 21, 2017

benjamingeer added this to the Beta Release milestone Mar 21, 2017

benjamingeer self-assigned this Mar 21, 2017

kilchenmann added the in-progress label Mar 21, 2017

Benjamin Geer added 4 commits March 21, 2017 11:50

refactor (webapi): Clean up code for pull request.

9a9ab91

Merge branch 'develop' into wip/performance-debugging

df80438

style (webapi): Reformat comment.

cc1de5b

fix (webapi): Set profile-queries to false by default.

524b41f

benjamingeer requested a review from tobiasschweizer March 21, 2017 11:18

benjamingeer assigned tobiasschweizer Mar 21, 2017

benjamingeer added the ready-for-review label Mar 21, 2017

fix (webapi): Comment out another use of MessageUtil.toSource().

56bde7d

benjamingeer mentioned this pull request Mar 22, 2017

Rework the Knora development with IntelliJ documentation #458

Merged

tobiasschweizer reviewed Mar 23, 2017

View reviewed changes

tobiasschweizer approved these changes Mar 23, 2017

View reviewed changes

benjamingeer merged commit b78086e into develop Mar 23, 2017

kilchenmann removed in-progress labels Mar 23, 2017

benjamingeer deleted the wip/performance-debugging branch March 23, 2017 11:25

benjamingeer mentioned this pull request Jul 5, 2019

Ben's PR history #571

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix performance problem at login #464

Fix performance problem at login #464

benjamingeer commented Mar 21, 2017 •

edited

Loading

tobiasschweizer commented Mar 23, 2017

benjamingeer commented Mar 23, 2017

tobiasschweizer Mar 23, 2017

benjamingeer Mar 23, 2017

benjamingeer Mar 23, 2017

tobiasschweizer Mar 23, 2017

benjamingeer Mar 23, 2017

tobiasschweizer Mar 23, 2017

benjamingeer Mar 23, 2017

tobiasschweizer Mar 23, 2017

benjamingeer Mar 23, 2017

tobiasschweizer Mar 23, 2017

benjamingeer Mar 23, 2017

tobiasschweizer commented Mar 23, 2017

benjamingeer commented Mar 23, 2017

tobiasschweizer commented Mar 23, 2017 •

edited

Loading

benjamingeer commented Mar 23, 2017

tobiasschweizer commented Mar 23, 2017

benjamingeer commented Mar 23, 2017

tobiasschweizer left a comment

tobiasschweizer commented Mar 23, 2017

benjamingeer commented Mar 23, 2017

subotic commented Mar 23, 2017

subotic commented Mar 23, 2017

Fix performance problem at login #464

Fix performance problem at login #464

Conversation

benjamingeer commented Mar 21, 2017 • edited Loading

tobiasschweizer commented Mar 23, 2017

benjamingeer commented Mar 23, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tobiasschweizer commented Mar 23, 2017

benjamingeer commented Mar 23, 2017

tobiasschweizer commented Mar 23, 2017 • edited Loading

benjamingeer commented Mar 23, 2017

tobiasschweizer commented Mar 23, 2017

benjamingeer commented Mar 23, 2017

tobiasschweizer left a comment

Choose a reason for hiding this comment

tobiasschweizer commented Mar 23, 2017

benjamingeer commented Mar 23, 2017

subotic commented Mar 23, 2017

subotic commented Mar 23, 2017

benjamingeer commented Mar 21, 2017 •

edited

Loading

tobiasschweizer commented Mar 23, 2017 •

edited

Loading