public inbox for [email protected]  

help / color / mirror / Atom feed

From: Surinder Kumar <[email protected]>
To: Dave Page <[email protected]>
Cc: Harshal Dhumal <[email protected]>
Cc: pgadmin-hackers <[email protected]>
Subject: Re: pgAdmin 4 commit: Cleanup handling of default/null values when data edi
Date: Mon, 29 May 2017 11:38:36 +0530
Message-ID: <CAM5-9D8+ExBUTaZeVkdH4hW54j1RKwF8x49oJtQC1fMK8nDOHw@mail.gmail.com> (raw)
In-Reply-To: <CA+OCxozbEZVuUO13uL65vRrhi0+Tfs7CAofYyqZEe73X9W0HVw@mail.gmail.com>
References: <[email protected]>
	<CAFiP3vwkX6N+6P21YmPZ6HgrZGjmCLVQWdqdXnzpdi0XVsirwg@mail.gmail.com>
	<CA+OCxozbEZVuUO13uL65vRrhi0+Tfs7CAofYyqZEe73X9W0HVw@mail.gmail.com>
List-Unsubscribe:  <mailto:[email protected]?body=unsub%20pgadmin-hackers>

Hi Dave,

The grid was being re-rendered after add new row and copy/paste to add a
blank row in the end of grid, but in case of copy/paste batch operation it
should run once, so that code is moved out of addNewRow(...) and put into a
function grid.addBlankRow() and called separately for copy/paste after
batch operation is completed.

Now copy/paste with 10k records took 2 seconds.

Please find attached patch.


On Mon, May 29, 2017 at 5:19 AM, Dave Page <[email protected]> wrote:

> On Sun, May 28, 2017 at 1:25 PM, Harshal Dhumal
> <[email protected]> wrote:
> > Hi,
> >
> > This commit has some performance issues with row paste functionality.
> > For 2K copied rows with 3 columns (2 integer and one null column) it took
> > near about 10 seconds to complete paste operation. And entire application
> > becomes unresponsive for those 10 seconds.
> >
> > This is mainly because for each single pasted row entire grid is
> re-rendered
> > ( is what I see in code).
> > Ideally grid should be re-rendered only once after all rows are provided
> to
> > grid.
> >
> > below code snippet from _paste_rows function
> >
> > _.each(copied_rows, function(row) {
> >     var new_row = arr_to_object(row);
> >     new_row.is_row_copied = true;
> >     row = new_row;
> >     self.temp_new_rows.push(count);
> >     grid.onAddNewRow.notify(
> >       {item: new_row, column: self.columns[0] , grid:grid}
> >     )
> >     grid.setSelectedRows([]);
> >     count++;
> > });
> >
> > The statement
> >
> > grid.onAddNewRow.notify(
> >       {item: new_row, column: self.columns[0] , grid:grid}
> >     )
> >
> > causes grid to re-render (as we listener on onAddNewRow event where we
> > re-render the grid)
>
> Copying that number of rows is an extreme case of course, but still...
> Is there an alternative way to batch notify?
>
> --
> Dave Page
> Blog: http://pgsnake.blogspot.com
> Twitter: @pgsnake
>
> EnterpriseDB UK: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>


-- 
Sent via pgadmin-hackers mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgadmin-hackers


Attachments:

  [application/octet-stream] render_grid_after_batch_copy_paste_rows.patch (2.0K, 3-render_grid_after_batch_copy_paste_rows.patch)
  download | inline diff:
diff --git a/web/pgadmin/tools/sqleditor/templates/sqleditor/js/sqleditor.js b/web/pgadmin/tools/sqleditor/templates/sqleditor/js/sqleditor.js
index 7d51193..597c436 100644
--- a/web/pgadmin/tools/sqleditor/templates/sqleditor/js/sqleditor.js
+++ b/web/pgadmin/tools/sqleditor/templates/sqleditor/js/sqleditor.js
@@ -910,6 +910,14 @@ define(
           $("#btn-save").prop('disabled', false);
         }.bind(editor_data));
 
+        grid.addBlankRow = function() {
+          // Add a blank row in the end of grid
+          this.setData(this.getData(), true);
+          this.updateRowCount();
+          this.invalidateAllRows();
+          this.render();
+        };
+
         // Listener function which will be called when user adds new rows
         grid.onAddNewRow.subscribe(function (e, args) {
           // self.handler.data_store.added will holds all the newly added rows/data
@@ -941,10 +949,9 @@ define(
           grid.render();
 
           // Add a blank row after add row
-          grid.setData(new_collection, true);
-          grid.updateRowCount();
-          grid.invalidateAllRows();
-          grid.render();
+          if (!args.is_copy_row) {
+            grid.addBlankRow();
+          }
 
           // Enable save button
           $("#btn-save").prop('disabled', false);
@@ -3171,11 +3178,17 @@ define(
                   row = new_row;
                   self.temp_new_rows.push(count);
                   grid.onAddNewRow.notify(
-                    {item: new_row, column: self.columns[0] , grid:grid}
+                    { item: new_row,
+                      column: self.columns[0],
+                      grid: grid,
+                      is_copy_row: true
+                    }
                   )
-                  grid.setSelectedRows([]);
                   count++;
               });
+              // Add a blank row after copy/paste row
+              grid.addBlankRow();
+              grid.setSelectedRows([]);
             }
         },
 

view thread (5+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected]
  Subject: Re: pgAdmin 4 commit: Cleanup handling of default/null values when data edi
  In-Reply-To: <CAM5-9D8+ExBUTaZeVkdH4hW54j1RKwF8x49oJtQC1fMK8nDOHw@mail.gmail.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox